xiangfu0 commented on code in PR #17998:
URL: https://github.com/apache/pinot/pull/17998#discussion_r3013062128
##########
pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/TextIndexConfigTest.java:
##########
@@ -122,6 +116,15 @@ public void withSomeData()
assertEquals(config.getLuceneMaxBufferSizeMB(), 1024, "Unexpected
luceneMaxBufferSize");
}
+ @Test
+ public void withLegacyNativeFstTypeIgnored()
Review Comment:
Done. Removed `withLegacyNativeFstTypeIgnored` from TextIndexConfigTest and
the corresponding `withLegacyLuceneTypeIgnored`/`withLegacyNativeTypeIgnored`
from FstIndexConfigTest.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/FSTIndexHandler.java:
##########
@@ -105,8 +114,22 @@ public void updateIndices(SegmentDirectory.Writer
segmentWriter)
// Remove indices not set in table config any more
String segmentName = _segmentDirectory.getSegmentMetadata().getName();
Set<String> columnsToAddIdx = new HashSet<>(_columnsToAddIdx);
+ Set<String> legacyNativeColumns =
getColumnsWithLegacyNativeFstIndex(segmentWriter);
+ for (String column : legacyNativeColumns) {
+ LOGGER.info("Removing legacy native FST index from segment: {}, column:
{}", segmentName, column);
+ segmentWriter.removeIndex(column, StandardIndexes.fst());
+ if (columnsToAddIdx.remove(column)) {
+ ColumnMetadata columnMetadata =
_segmentDirectory.getSegmentMetadata().getColumnMetadataFor(column);
+ if (shouldCreateFSTIndex(columnMetadata)) {
+ createFSTIndexForColumn(segmentWriter, columnMetadata);
+ }
+ }
+ }
Set<String> existingColumns =
segmentWriter.toSegmentDirectory().getColumnsWithIndex(StandardIndexes.fst());
for (String column : existingColumns) {
+ if (legacyNativeColumns.contains(column)) {
+ continue;
+ }
Review Comment:
Good catch. Restructured: legacy indexes are now removed before fetching
`existingColumns`, so they no longer appear in the set. This eliminates the
`legacyNativeColumns.contains()` guard and the `recreatedLegacyNativeColumns`
tracking. Applied the same simplification to TextIndexHandler.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
