somandal commented on code in PR #9333:
URL: https://github.com/apache/pinot/pull/9333#discussion_r974708484
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/TextIndexHandler.java:
##########
@@ -174,18 +175,46 @@ private void
createTextIndexForColumn(SegmentDirectory.Writer segmentWriter, Col
// segmentDirectory is indicated to us by SegmentDirectoryPaths, we create
lucene index there. There is no
// further need to move around the lucene index directory since it is
created with correct directory structure
// based on segmentVersion.
- try (ForwardIndexReader forwardIndexReader =
LoaderUtils.getForwardIndexReader(segmentWriter, columnMetadata);
- ForwardIndexReaderContext readerContext =
forwardIndexReader.createContext();
- TextIndexCreator textIndexCreator =
indexCreatorProvider.newTextIndexCreator(IndexCreationContext.builder()
-
.withColumnMetadata(columnMetadata).withIndexDir(segmentDirectory).build().forTextIndex(_fstType,
true))) {
- if (columnMetadata.isSingleValue()) {
- processSVField(segmentWriter, hasDictionary, forwardIndexReader,
readerContext, textIndexCreator, numDocs,
- columnMetadata);
+ try (TextIndexCreator textIndexCreator =
indexCreatorProvider.newTextIndexCreator(
+
IndexCreationContext.builder().withColumnMetadata(columnMetadata).withIndexDir(segmentDirectory).build()
+ .forTextIndex(_fstType, true))) {
+ boolean forwardIndexDisabled = !segmentWriter.hasIndexFor(columnName,
ColumnIndexType.FORWARD_INDEX);
+ if (forwardIndexDisabled) {
+ try (Dictionary dictionary = LoaderUtils.getDictionary(segmentWriter,
columnMetadata)) {
+ // Create the text index if the dictionary length is 1 as this is
for a default column (i.e. newly added
+ // column). For existing columns it is not possible to create the
text index without forward index
+ Preconditions.checkState(dictionary.length() == 1,
String.format("Creating text index for forward index "
Review Comment:
Hi @Jackie-Jiang what do you mean by directly throwing exception?
Without adding this special handling, creation of these indexes for default
columns will fail. Today when a new column is added, a default forward index is
created. On the reload path I'm skipping the creation of the forward index when
this feature is disabled for such default columns. I still create a dictionary
though. I do want to create other indices since queries may fail if such
segments are accessed and the expected indexes don't exist, right? Or am I
missing something here? (i did see the segment preprocessor tests fail when I
didn't do special handling for the inverted index for example since the default
handlers directly access the forward index to create the auxiliary index)
--
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
