From 5984b639e2ef5b673056d077457623a74ff19843 Mon Sep 17 00:00:00 2001 From: Akanksha Kedia Date: Fri, 10 Jul 2026 11:39:27 +0530 Subject: [PATCH 1/4] Add unit tests for IFSTIndexHandler index lifecycle IFSTIndexHandler (case-insensitive FST) had no unit test coverage. Adds IFSTIndexHandlerTest with 5 tests covering: - Column dropped from config triggers index removal - updateIndices removes index for dropped column - New column with metadata present triggers index creation - Column in config but metadata absent skips index creation - Index already present and in config requires no update Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../invertedindex/IFSTIndexHandlerTest.java | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java new file mode 100644 index 000000000000..2190fac8631d --- /dev/null +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java @@ -0,0 +1,172 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.segment.index.loader.invertedindex; + +import java.util.Map; +import java.util.Set; +import org.apache.pinot.segment.spi.ColumnMetadata; +import org.apache.pinot.segment.spi.index.FieldIndexConfigs; +import org.apache.pinot.segment.spi.index.FstIndexConfig; +import org.apache.pinot.segment.spi.index.StandardIndexes; +import org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl; +import org.apache.pinot.segment.spi.store.SegmentDirectory; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.Schema; +import org.testng.annotations.Test; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + + +/** + * Unit tests for {@link IFSTIndexHandler}. + * + *

IFST (case-Insensitive FST) indexes share the same index lifecycle as FST indexes but + * use a separate index type ({@link StandardIndexes#ifst()}) and file extension. + * + *

Covers: + *

+ */ +public class IFSTIndexHandlerTest { + private static final String COLUMN = "category"; + + // --------------------------------------------------------------------------- + // Column removed from config + // --------------------------------------------------------------------------- + + @Test + public void testNeedUpdateReturnsTrueWhenColumnRemovedFromConfig() + throws Exception { + // Column has an IFST index on disk but the new config has no IFST index for it. + SegmentDirectory segmentDirectory = mockSegmentDirectory(COLUMN); + SegmentDirectory.Reader reader = mockReader(segmentDirectory); + + IFSTIndexHandler handler = new IFSTIndexHandler(segmentDirectory, Map.of(), + mock(TableConfig.class), mock(Schema.class)); + + assertTrue(handler.needUpdateIndices(reader), + "Rebuild expected to remove index when column is dropped from IFST index config"); + } + + @Test + public void testUpdateIndicesRemovesIndexWhenColumnDroppedFromConfig() + throws Exception { + SegmentDirectory segmentDirectory = mockSegmentDirectory(COLUMN); + SegmentDirectory.Writer writer = mock(SegmentDirectory.Writer.class); + when(writer.toSegmentDirectory()).thenReturn(segmentDirectory); + + IFSTIndexHandler handler = new IFSTIndexHandler(segmentDirectory, Map.of(), + mock(TableConfig.class), mock(Schema.class)); + handler.updateIndices(writer); + + verify(writer).removeIndex(COLUMN, StandardIndexes.ifst()); + } + + // --------------------------------------------------------------------------- + // New column added to config + // --------------------------------------------------------------------------- + + @Test + public void testNeedUpdateReturnsTrueWhenNewColumnAdded() + throws Exception { + // Column is in config but has no existing IFST index — metadata present. + SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); + when(segmentMetadata.getName()).thenReturn("testSegment"); + when(segmentMetadata.getColumnMetadataFor(COLUMN)).thenReturn(mock(ColumnMetadata.class)); + + SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); + when(segmentDirectory.getSegmentMetadata()).thenReturn(segmentMetadata); + when(segmentDirectory.getColumnsWithIndex(StandardIndexes.ifst())).thenReturn(Set.of()); + + SegmentDirectory.Reader reader = mock(SegmentDirectory.Reader.class); + when(reader.toSegmentDirectory()).thenReturn(segmentDirectory); + + assertTrue(createHandler(segmentDirectory).needUpdateIndices(reader), + "New IFST index expected for column added to config with metadata present"); + } + + @Test + public void testNeedUpdateReturnsFalseWhenColumnMetadataAbsent() + throws Exception { + // Column is in config but getColumnMetadataFor returns null — not yet in segment. + SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); + when(segmentMetadata.getName()).thenReturn("testSegment"); + when(segmentMetadata.getColumnMetadataFor(COLUMN)).thenReturn(null); + + SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); + when(segmentDirectory.getSegmentMetadata()).thenReturn(segmentMetadata); + when(segmentDirectory.getColumnsWithIndex(StandardIndexes.ifst())).thenReturn(Set.of()); + + SegmentDirectory.Reader reader = mock(SegmentDirectory.Reader.class); + when(reader.toSegmentDirectory()).thenReturn(segmentDirectory); + + assertFalse(createHandler(segmentDirectory).needUpdateIndices(reader), + "No update expected when column metadata is absent"); + } + + // --------------------------------------------------------------------------- + // No update when index is already up-to-date + // --------------------------------------------------------------------------- + + @Test + public void testNeedUpdateReturnsFalseWhenIndexUpToDate() + throws Exception { + // Column has an IFST index and is still in config — nothing to do. + SegmentDirectory segmentDirectory = mockSegmentDirectory(COLUMN); + SegmentDirectory.Reader reader = mockReader(segmentDirectory); + + assertFalse(createHandler(segmentDirectory).needUpdateIndices(reader), + "No update expected when IFST index is present and matches config"); + } + + // --------------------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------------------- + + private static IFSTIndexHandler createHandler(SegmentDirectory segmentDirectory) { + FieldIndexConfigs fieldIndexConfigs = + new FieldIndexConfigs.Builder().add(StandardIndexes.ifst(), new FstIndexConfig()).build(); + return new IFSTIndexHandler(segmentDirectory, Map.of(COLUMN, fieldIndexConfigs), + mock(TableConfig.class), mock(Schema.class)); + } + + private static SegmentDirectory mockSegmentDirectory(String column) { + SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); + when(segmentMetadata.getName()).thenReturn("testSegment"); + + SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); + when(segmentDirectory.getSegmentMetadata()).thenReturn(segmentMetadata); + when(segmentDirectory.getColumnsWithIndex(StandardIndexes.ifst())).thenReturn(Set.of(column)); + return segmentDirectory; + } + + private static SegmentDirectory.Reader mockReader(SegmentDirectory segmentDirectory) { + SegmentDirectory.Reader reader = mock(SegmentDirectory.Reader.class); + when(reader.toSegmentDirectory()).thenReturn(segmentDirectory); + return reader; + } +} From 12f003b85cfd63ca02dafe752d640a1ba6df1b0f Mon Sep 17 00:00:00 2001 From: Akanksha Kedia Date: Sat, 11 Jul 2026 20:01:21 +0530 Subject: [PATCH 2/4] Fix IFSTIndexHandlerTest: stub getDataType/hasDictionary/isSingleValue on ColumnMetadata mock checkUnsupportedOperationsForIFSTIndex() validates that the column is STRING, dictionary-encoded, and single-value before creating the index. The mock returned null for getDataType(), causing null != STRING to throw UnsupportedOperationException instead of returning true. --- .../index/loader/invertedindex/IFSTIndexHandlerTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java index 2190fac8631d..fbc4b5444ddd 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java @@ -27,6 +27,7 @@ import org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl; import org.apache.pinot.segment.spi.store.SegmentDirectory; import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.FieldSpec; import org.apache.pinot.spi.data.Schema; import org.testng.annotations.Test; @@ -96,7 +97,12 @@ public void testNeedUpdateReturnsTrueWhenNewColumnAdded() // Column is in config but has no existing IFST index — metadata present. SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); when(segmentMetadata.getName()).thenReturn("testSegment"); - when(segmentMetadata.getColumnMetadataFor(COLUMN)).thenReturn(mock(ColumnMetadata.class)); + ColumnMetadata columnMetadata = mock(ColumnMetadata.class); + when(columnMetadata.getColumnName()).thenReturn(COLUMN); + when(columnMetadata.getDataType()).thenReturn(FieldSpec.DataType.STRING); + when(columnMetadata.hasDictionary()).thenReturn(true); + when(columnMetadata.isSingleValue()).thenReturn(true); + when(segmentMetadata.getColumnMetadataFor(COLUMN)).thenReturn(columnMetadata); SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); when(segmentDirectory.getSegmentMetadata()).thenReturn(segmentMetadata); From 761113e0e1881715798d0aefca2cbc0f2afc9b67 Mon Sep 17 00:00:00 2001 From: Akanksha Kedia Date: Sat, 11 Jul 2026 21:20:00 +0530 Subject: [PATCH 3/4] test(segment): fix IFSTIndexHandlerTest mocks to satisfy BaseIndexHandler preconditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stub getTotalDocs()→1 and getAllColumns()→TreeSet in all SegmentMetadataImpl mocks so the BaseIndexHandler constructor precondition check does not throw. --- .../index/loader/invertedindex/IFSTIndexHandlerTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java index fbc4b5444ddd..049ced3ffc17 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.Set; +import java.util.TreeSet; import org.apache.pinot.segment.spi.ColumnMetadata; import org.apache.pinot.segment.spi.index.FieldIndexConfigs; import org.apache.pinot.segment.spi.index.FstIndexConfig; @@ -97,6 +98,8 @@ public void testNeedUpdateReturnsTrueWhenNewColumnAdded() // Column is in config but has no existing IFST index — metadata present. SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); when(segmentMetadata.getName()).thenReturn("testSegment"); + when(segmentMetadata.getTotalDocs()).thenReturn(1); + when(segmentMetadata.getAllColumns()).thenReturn(new TreeSet<>(Set.of(COLUMN))); ColumnMetadata columnMetadata = mock(ColumnMetadata.class); when(columnMetadata.getColumnName()).thenReturn(COLUMN); when(columnMetadata.getDataType()).thenReturn(FieldSpec.DataType.STRING); @@ -121,6 +124,8 @@ public void testNeedUpdateReturnsFalseWhenColumnMetadataAbsent() // Column is in config but getColumnMetadataFor returns null — not yet in segment. SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); when(segmentMetadata.getName()).thenReturn("testSegment"); + when(segmentMetadata.getTotalDocs()).thenReturn(1); + when(segmentMetadata.getAllColumns()).thenReturn(new TreeSet<>(Set.of(COLUMN))); when(segmentMetadata.getColumnMetadataFor(COLUMN)).thenReturn(null); SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); @@ -163,6 +168,8 @@ private static IFSTIndexHandler createHandler(SegmentDirectory segmentDirectory) private static SegmentDirectory mockSegmentDirectory(String column) { SegmentMetadataImpl segmentMetadata = mock(SegmentMetadataImpl.class); when(segmentMetadata.getName()).thenReturn("testSegment"); + when(segmentMetadata.getTotalDocs()).thenReturn(1); + when(segmentMetadata.getAllColumns()).thenReturn(new TreeSet<>(Set.of(column))); SegmentDirectory segmentDirectory = mock(SegmentDirectory.class); when(segmentDirectory.getSegmentMetadata()).thenReturn(segmentMetadata); From 06b5a731b4d4d5ae6695d6057a2db2c90751cf67 Mon Sep 17 00:00:00 2001 From: Akanksha Kedia Date: Tue, 14 Jul 2026 08:50:10 +0530 Subject: [PATCH 4/4] style: convert IFSTIndexHandlerTest class doc to JEP 467 Markdown style; drop section dividers --- .../invertedindex/IFSTIndexHandlerTest.java | 41 +++++-------------- 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java index 049ced3ffc17..bd5e1991f245 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/IFSTIndexHandlerTest.java @@ -39,27 +39,20 @@ import static org.testng.Assert.assertTrue; -/** - * Unit tests for {@link IFSTIndexHandler}. - * - *

IFST (case-Insensitive FST) indexes share the same index lifecycle as FST indexes but - * use a separate index type ({@link StandardIndexes#ifst()}) and file extension. - * - *

Covers: - *

    - *
  • Index removal when a column is dropped from the IFST index config
    • - *
  • New index creation when a column is added to the config
    • - *
  • No update required when the index is already present and matches config
    • - *
  • No update when column metadata is absent (column not in segment)
    • - *
- */ +/// Unit tests for [IFSTIndexHandler]. +/// +/// IFST (case-insensitive FST) indexes share the same index lifecycle as FST indexes but +/// use a separate index type ([StandardIndexes#ifst()]) and file extension. +/// +/// Covers: +/// - needUpdateIndices returns true when a column is dropped from the IFST index config +/// - updateIndices removes the on-disk index when a column is dropped from config +/// - needUpdateIndices returns true when a new column is added to the config +/// - needUpdateIndices returns false when the index is already present and matches config +/// - needUpdateIndices returns false when column metadata is absent (column not in segment) public class IFSTIndexHandlerTest { private static final String COLUMN = "category"; - // --------------------------------------------------------------------------- - // Column removed from config - // --------------------------------------------------------------------------- - @Test public void testNeedUpdateReturnsTrueWhenColumnRemovedFromConfig() throws Exception { @@ -88,10 +81,6 @@ public void testUpdateIndicesRemovesIndexWhenColumnDroppedFromConfig() verify(writer).removeIndex(COLUMN, StandardIndexes.ifst()); } - // --------------------------------------------------------------------------- - // New column added to config - // --------------------------------------------------------------------------- - @Test public void testNeedUpdateReturnsTrueWhenNewColumnAdded() throws Exception { @@ -139,10 +128,6 @@ public void testNeedUpdateReturnsFalseWhenColumnMetadataAbsent() "No update expected when column metadata is absent"); } - // --------------------------------------------------------------------------- - // No update when index is already up-to-date - // --------------------------------------------------------------------------- - @Test public void testNeedUpdateReturnsFalseWhenIndexUpToDate() throws Exception { @@ -154,10 +139,6 @@ public void testNeedUpdateReturnsFalseWhenIndexUpToDate() "No update expected when IFST index is present and matches config"); } - // --------------------------------------------------------------------------- - // Helpers - // --------------------------------------------------------------------------- - private static IFSTIndexHandler createHandler(SegmentDirectory segmentDirectory) { FieldIndexConfigs fieldIndexConfigs = new FieldIndexConfigs.Builder().add(StandardIndexes.ifst(), new FstIndexConfig()).build();