Jackie-Jiang commented on code in PR #9333:
URL: https://github.com/apache/pinot/pull/9333#discussion_r976061367


##########
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:
   For the MV default column, since we don't generate sorted index for it, we 
can consider directly creating inverted index in the default column handler.
   Essentially we should try to keep the behavior the same for reloaded segment 
and segment generated without the data for the default column. That way we 
don't need any special handling for other index handlers, and the logic will be 
cleaner.



-- 
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

Reply via email to