somandal commented on code in PR #9740:
URL: https://github.com/apache/pinot/pull/9740#discussion_r1018627157


##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java:
##########
@@ -1903,19 +1908,76 @@ public void 
testForwardIndexDisabledOnExistingColumnDictEncoded()
     ColumnMetadata columnMetadata = 
segmentMetadata.getColumnMetadataFor(EXISTING_STRING_COL_DICT);
     assertNotNull(columnMetadata);
 
-    SegmentDirectory segmentDirectory = 
SegmentDirectoryLoaderRegistry.getDefaultSegmentDirectoryLoader()
-        .load(_indexDir.toURI(),
-            new 
SegmentDirectoryLoaderContext.Builder().setSegmentDirectoryConfigs(_configuration).build());
-    SegmentPreProcessor v3Processor =
-        new SegmentPreProcessor(segmentDirectory, _indexLoadingConfig, 
_newColumnsSchemaWithForwardIndexDisabled);
-    expectThrows(RuntimeException.class, v3Processor::process);
+    createAndValidateIndex(ColumnIndexType.FORWARD_INDEX, 
EXISTING_STRING_COL_DICT, 9, 4,
+        _newColumnsSchemaWithForwardIndexDisabled, false, true, false, 26, 
true, 0, null, true, DataType.STRING,
+        100000);
 
     constructV1Segment(Collections.emptyList(), Collections.emptyList(), 
Collections.emptyList());
-    segmentDirectory = 
SegmentDirectoryLoaderRegistry.getDefaultSegmentDirectoryLoader().load(_indexDir.toURI(),
-        new 
SegmentDirectoryLoaderContext.Builder().setSegmentDirectoryConfigs(_configuration).build());
-    SegmentPreProcessor v1Processor =
-        new SegmentPreProcessor(segmentDirectory, _indexLoadingConfig, 
_newColumnsSchemaWithForwardIndexDisabled);
-    expectThrows(RuntimeException.class, v1Processor::process);
+
+    createAndValidateIndex(ColumnIndexType.FORWARD_INDEX, 
EXISTING_STRING_COL_DICT, 9, 4,

Review Comment:
   done



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandler.java:
##########
@@ -112,20 +122,50 @@ public void updateIndices(SegmentDirectory.Writer 
segmentWriter, IndexCreatorPro
       Operation operation = entry.getValue();
 
       switch (operation) {
-        case CHANGE_RAW_INDEX_COMPRESSION_TYPE: {
-          rewriteRawForwardIndex(column, segmentWriter, indexCreatorProvider);
+        case DISABLE_FORWARD_INDEX_FOR_DICT_COLUMN: {
+          // Deletion of the forward index will be handled outside the index 
handler to ensure that other index
+          // handlers that need the forward index to construct their own 
indexes will have it available.
+          // The existing forward index must be in dictionary format for this 
to be a no-op.
+          _forwardIndexDisabledColumnsToCleanup.add(column);
+          break;
+        }
+        case DISABLE_FORWARD_INDEX_FOR_RAW_COLUMN: {
+          // The forward index has been disabled for a column which has a 
noDictionary based forward index. A dictionary
+          // and inverted index need to be created before we can delete the 
forward index. We create a dictionary here,
+          // but let the InvertedIndexHandler handle the creation of the 
inverted index. We create a temporary
+          // forward index here which is dictionary based and allow the post 
deletion step handle the actual deletion
+          // of the forward index.
+          createDictBasedForwardIndex(column, segmentWriter, 
indexCreatorProvider);
+          Preconditions.checkState(segmentWriter.hasIndexFor(column, 
ColumnIndexType.FORWARD_INDEX),
+              String.format("Temporary forward index was not created for 
column: %s", column));
+          _forwardIndexDisabledColumnsToCleanup.add(column);

Review Comment:
   done



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