jackjlli commented on PR #12838: URL: https://github.com/apache/pinot/pull/12838#issuecomment-2046083662
> Do we allow disabling forward index for consuming segment? Can we add a test to verify if it works? With the change in this current PR no we won't allow disabling forward index for consuming segment as it will fail on table creation. And `columnMajorSegmentBuilderEnabled` is set to true by default in the previous [PR](https://github.com/apache/pinot/pull/12770). And in fact, columnar segment generation won't work with consuming segment whose column has forward index disabled. Even without adding the validation check, the columnar segment generation would still fail (check [this line of code](https://github.com/apache/pinot/blob/master/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/readers/PinotSegmentColumnReader.java#L48)). The purpose of this PR is to fail fast instead of waiting for a segment commit request. Regarding tests, I think the ones in `TableConfigUtilsTest` would be sufficient as in this PR the failure would be detected during table creation? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
