You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xi...@apache.org on 2026/09/17 01:08:06 UTC

(pinot) branch master updated: Drop star-tree indexes left unreadable by a dimension encoding change (#19349)

This is an automated email from the ASF dual-hosted git repository.

xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 3764aea534e Drop star-tree indexes left unreadable by a dimension encoding change (#19349)
3764aea534e is described below

commit 3764aea534e9b2cc0dffe0ecd0c50f31d5d35b7e
Author: Gonzalo Ortiz Jaureguizar <go...@users.noreply.github.com>
AuthorDate: Thu Sep 17 03:08:00 2026 +0200

    Drop star-tree indexes left unreadable by a dimension encoding change (#19349)
    
    * Drop star-tree indexes left unreadable by a dimension encoding change
    
    A star-tree stores its dimension values as dictionary ids in a fixed-bit
    forward index whose bit width is read from the main column metadata at load
    time. Moving a star-tree dimension column to `noDictionaryColumns` makes
    `ForwardIndexHandler` re-encode that column to raw, but with
    `enableDynamicStarTreeCreation` disabled `SegmentPreProcessor` leaves the
    star-tree untouched. The segment then becomes permanently unloadable:
    `StarTreeLoaderUtils` asks `FixedBitIntReader` for a reader over the now
    unavailable bit width and gets a bare `IllegalStateException`, so the segment
    goes to ERROR on every load, e.g. on any restart that reloads it.
    
    The re-encoding is persisted, so an affected segment is already inconsistent on
    disk and `needProcess()` no longer reports anything to do, which makes a plain
    reload fail in exactly the same way.
    
    - `SegmentPreProcessor` now removes star-trees whose dimension column lost its
      dictionary even when `enableDynamicStarTreeCreation` is disabled. Removing
      star-trees only deletes files, unlike the rebuild that flag guards against.
      When the flag is enabled the regular flow already rebuilds them, since the
      split order no longer matches the builder configs.
    - `StarTreeLoaderUtils` skips an unreadable star-tree with a warning instead of
      failing the whole segment load, covering segments whose pre-processing is
      skipped.
    - `FixedBitIntReader` now reports the offending number of bits.
    
    Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
    
    * Verify star-tree survives a RAW forward index with separated dictionary
    
    Address review feedback on #19349:
    - Reflow the unloadableDimensions ternary.
    - Add testStarTreeDimensionConvertedToRawWithSeparatedDictionary, covering the
      RAW forward index with separated dictionary configuration added in #19153. The
      dimension keeps its dictionary, so the star-tree stays loadable and must not be
      dropped by the new repair path.
    
    * Assert the pre-process gates in the star-tree encoding-drift tests
    
    Address review feedback on the new tests:
    
    - testStarTreeDimensionConvertedToRawWithSeparatedDictionary now asserts
      needProcess() before and after processing. The star-tree must not be
      treated as stale, and a second round must be a no-op across every
      handler, not just the star-tree one.
    - testStarTreeDimensionConvertedToNoDictionaryWithoutPreprocess now goes
      through ImmutableSegmentLoader#needPreprocess with skipSegmentPreprocess
      set, which is the gate that actually skips the repair, instead of just
      passing needPreprocess = false to the loader. It also asserts the
      star-tree metadata is still on disk, so the test distinguishes a
      star-tree skipped at load from one that was removed.
    
    * Adapt the skip-preprocess test to the current loader semantics
    
    Two changes on master land on this test:
    
    - #19391 made IndexLoadingConfig snapshot skipSegmentPreprocess instead of
      reading it live from the table config, so setting the flag now needs a
      fresh IndexLoadingConfig rather than an in-place edit.
    - #19394 gated ImmutableSegmentLoader#load on needPreprocess(), so the
      loader can be asked to pre-process and will still honour the flag.
    
    The test now builds a separate config for the flag, asserts needPreprocess()
    flips from true to false across it, and lets the loader decide, which is the
    path the server takes.
    
    ---------
    
    Co-authored-by: Gonzalo Ortiz <19...@users.noreply.github.com>
    Co-authored-by: Claude Opus 5 <no...@anthropic.com>
---
 .../local/io/reader/impl/FixedBitIntReader.java    |   3 +-
 .../segment/index/loader/SegmentPreProcessor.java  |  32 ++-
 .../local/startree/StarTreeBuilderUtils.java       |  34 +++
 .../startree/v2/store/StarTreeLoaderUtils.java     |  19 +-
 .../index/loader/SegmentPreProcessorTest.java      | 272 +++++++++++++++++++++
 5 files changed, 351 insertions(+), 9 deletions(-)

diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/reader/impl/FixedBitIntReader.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/reader/impl/FixedBitIntReader.java
index 3308566bd71..12091b6b13e 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/reader/impl/FixedBitIntReader.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/reader/impl/FixedBitIntReader.java
@@ -106,7 +106,8 @@ public abstract class FixedBitIntReader {
       case 31:
         return new Bit31Reader(dataBuffer);
       default:
-        throw new IllegalStateException();
+        throw new IllegalStateException("Illegal number of bits per value: " + numBitsPerValue + ", must be within 1 "
+            + "and 31");
     }
   }
 
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
index e4d74d336b2..f8dbc0dddce 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
@@ -295,16 +295,18 @@ public class SegmentPreProcessor implements AutoCloseable {
   }
 
   private boolean needProcessStarTrees() {
+    SegmentMetadataImpl segmentMetadata = _segmentDirectory.getSegmentMetadata();
+    List<StarTreeV2Metadata> starTreeMetadataList = segmentMetadata.getStarTreeV2MetadataList();
     // Check if there is need to create/modify/remove star-trees.
     if (!_indexLoadingConfig.isEnableDynamicStarTreeCreation()) {
-      return false;
+      // Star-trees left unreadable by a column encoding change are still removed, see processStarTrees().
+      return starTreeMetadataList != null && !StarTreeBuilderUtils.findUnloadableDimensions(starTreeMetadataList,
+          segmentMetadata).isEmpty();
     }
 
-    SegmentMetadataImpl segmentMetadata = _segmentDirectory.getSegmentMetadata();
     List<StarTreeV2BuilderConfig> starTreeBuilderConfigs =
         StarTreeBuilderUtils.generateBuilderConfigs(_indexLoadingConfig.getStarTreeIndexConfigs(),
             _indexLoadingConfig.isEnableDefaultStarTree(), segmentMetadata);
-    List<StarTreeV2Metadata> starTreeMetadataList = segmentMetadata.getStarTreeV2MetadataList();
     // There are existing star-trees, but if they match the builder configs exactly,
     // then there is no need to generate the star-trees
 
@@ -397,19 +399,35 @@ public class SegmentPreProcessor implements AutoCloseable {
   private boolean processStarTrees(File indexDir,
       @Nullable SegmentOperationsThrottlerSet segmentOperationsThrottlerSet)
       throws Exception {
+    SegmentMetadataImpl segmentMetadata = _segmentDirectory.getSegmentMetadata();
+    String segmentName = segmentMetadata.getName();
+    List<StarTreeV2Metadata> starTreeMetadataList = segmentMetadata.getStarTreeV2MetadataList();
+
     if (!_indexLoadingConfig.isEnableDynamicStarTreeCreation()) {
-      return false;
+      // A star-tree whose dimension column is no longer dictionary-encoded (e.g. because the column was added to
+      // 'noDictionaryColumns' and re-encoded by the forward index handler above) cannot be read, and fails the whole
+      // segment load. Drop it even here: removing star-trees only deletes files, so unlike rebuilding them it is
+      // cheap enough to do with dynamic star-tree creation disabled. When it is enabled the star-trees are rebuilt by
+      // the regular flow below, because their split order no longer matches the builder configs.
+      Set<String> unloadableDimensions = starTreeMetadataList != null
+          ? StarTreeBuilderUtils.findUnloadableDimensions(starTreeMetadataList, segmentMetadata)
+          : Set.of();
+      if (unloadableDimensions.isEmpty()) {
+        return false;
+      }
+      LOGGER.warn("Removing star-trees from segment: {} because dimension columns: {} are no longer "
+              + "dictionary-encoded. Enable dynamic star-tree creation to have them rebuilt", segmentName,
+          unloadableDimensions);
+      StarTreeBuilderUtils.removeStarTrees(indexDir);
+      return true;
     }
 
-    SegmentMetadataImpl segmentMetadata = _segmentDirectory.getSegmentMetadata();
-    String segmentName = segmentMetadata.getName();
     List<StarTreeV2BuilderConfig> starTreeBuilderConfigs =
         StarTreeBuilderUtils.generateBuilderConfigs(_indexLoadingConfig.getStarTreeIndexConfigs(),
             _indexLoadingConfig.isEnableDefaultStarTree(), segmentMetadata);
 
     boolean shouldGenerateStarTree = !starTreeBuilderConfigs.isEmpty();
     boolean shouldRemoveStarTree = false;
-    List<StarTreeV2Metadata> starTreeMetadataList = segmentMetadata.getStarTreeV2MetadataList();
     if (starTreeMetadataList != null) {
       // There are existing star-trees
       if (!shouldGenerateStarTree) {
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/StarTreeBuilderUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/StarTreeBuilderUtils.java
index 72e73b4cfd8..465a5737f40 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/StarTreeBuilderUtils.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/StarTreeBuilderUtils.java
@@ -28,7 +28,9 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.TreeSet;
 import javax.annotation.Nullable;
 import org.apache.commons.configuration2.PropertiesConfiguration;
 import org.apache.commons.io.FileUtils;
@@ -36,6 +38,7 @@ import org.apache.pinot.common.request.Literal;
 import org.apache.pinot.common.request.context.ExpressionContext;
 import org.apache.pinot.segment.local.startree.v2.builder.StarTreeV2BuilderConfig;
 import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.segment.spi.ColumnMetadata;
 import org.apache.pinot.segment.spi.Constants;
 import org.apache.pinot.segment.spi.SegmentMetadata;
 import org.apache.pinot.segment.spi.index.startree.AggregationFunctionColumnPair;
@@ -283,6 +286,37 @@ public class StarTreeBuilderUtils {
     return false;
   }
 
+  /// Returns the first dimension of the given star-tree that the segment can no longer back with a dictionary
+  /// encoded forward index, or `null` if the star-tree is loadable.
+  ///
+  /// A star-tree stores its dimension values as dictionary ids in a fixed-bit forward index whose bit width is read
+  /// from the *main* column metadata at load time. Re-encoding a dimension column to raw (e.g. after adding it to
+  /// `noDictionaryColumns`) therefore leaves the star-tree unreadable, and loading the segment fails.
+  @Nullable
+  public static String findUnloadableDimension(StarTreeV2Metadata starTreeMetadata, SegmentMetadata segmentMetadata) {
+    for (String dimension : starTreeMetadata.getDimensionsSplitOrder()) {
+      ColumnMetadata columnMetadata = segmentMetadata.getColumnMetadataFor(dimension);
+      if (columnMetadata == null || !columnMetadata.hasDictionary()) {
+        return dimension;
+      }
+    }
+    return null;
+  }
+
+  /// Returns the dimensions that make the given star-trees unloadable, or an empty set if they are all loadable.
+  /// See [#findUnloadableDimension(StarTreeV2Metadata, SegmentMetadata)].
+  public static Set<String> findUnloadableDimensions(List<StarTreeV2Metadata> metadataList,
+      SegmentMetadata segmentMetadata) {
+    Set<String> dimensions = new TreeSet<>();
+    for (StarTreeV2Metadata starTreeMetadata : metadataList) {
+      String dimension = findUnloadableDimension(starTreeMetadata, segmentMetadata);
+      if (dimension != null) {
+        dimensions.add(dimension);
+      }
+    }
+    return dimensions;
+  }
+
   /// Returns `true` if the given star-tree builder configs are equal, `false` otherwise.
   public static boolean areStarTreeBuilderConfigListsEqual(List<StarTreeV2BuilderConfig> builderConfig1,
       List<StarTreeV2BuilderConfig> builderConfig2) {
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/v2/store/StarTreeLoaderUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/v2/store/StarTreeLoaderUtils.java
index 4e6ab15b49b..07e1a8c7d1f 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/v2/store/StarTreeLoaderUtils.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/v2/store/StarTreeLoaderUtils.java
@@ -27,6 +27,7 @@ import org.apache.pinot.segment.local.aggregator.ValueAggregatorFactory;
 import org.apache.pinot.segment.local.segment.index.forward.ForwardIndexReaderFactory;
 import org.apache.pinot.segment.local.segment.index.readers.forward.FixedBitSVForwardIndexReaderV2;
 import org.apache.pinot.segment.local.startree.OffHeapStarTree;
+import org.apache.pinot.segment.local.startree.StarTreeBuilderUtils;
 import org.apache.pinot.segment.spi.ColumnMetadata;
 import org.apache.pinot.segment.spi.datasource.DataSource;
 import org.apache.pinot.segment.spi.index.StandardIndexes;
@@ -42,10 +43,14 @@ import org.apache.pinot.segment.spi.store.SegmentDirectory;
 import org.apache.pinot.spi.data.FieldSpec;
 import org.apache.pinot.spi.data.FieldSpec.DataType;
 import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 
 /// The `StarTreeLoaderUtils` class provides utility methods to load star-tree indexes.
 public class StarTreeLoaderUtils {
+  private static final Logger LOGGER = LoggerFactory.getLogger(StarTreeLoaderUtils.class);
+
   private StarTreeLoaderUtils() {
   }
 
@@ -57,11 +62,23 @@ public class StarTreeLoaderUtils {
     int numStarTrees = starTreeMetadataList.size();
     List<StarTreeV2> starTrees = new ArrayList<>(numStarTrees);
     for (int i = 0; i < numStarTrees; i++) {
+      StarTreeV2Metadata starTreeMetadata = starTreeMetadataList.get(i);
+
+      // A star-tree is unreadable once one of its dimension columns loses its dictionary, which happens when the
+      // column is moved to 'noDictionaryColumns' without the star-tree being rebuilt. Skip it instead of failing the
+      // whole segment load. SegmentPreProcessor normally removes such star-trees, so reaching this point means the
+      // pre-processing was skipped for this segment.
+      String unloadableDimension = StarTreeBuilderUtils.findUnloadableDimension(starTreeMetadata, segmentMetadata);
+      if (unloadableDimension != null) {
+        LOGGER.warn("Skipping star-tree: {} in segment: {} because dimension column: {} is no longer "
+            + "dictionary-encoded", i, segmentMetadata.getName(), unloadableDimension);
+        continue;
+      }
+
       SegmentDirectory.Reader indexReader = segmentReader.getStarTreeIndexReader(i);
       // Load star-tree index
       StarTree starTree = new OffHeapStarTree(indexReader.getIndexFor(String.valueOf(i), StandardIndexes.inverted()));
 
-      StarTreeV2Metadata starTreeMetadata = starTreeMetadataList.get(i);
       int numDocs = starTreeMetadata.getNumDocs();
       Map<String, DataSource> dataSourceMap = new HashMap<>();
 
diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
index c7272504a37..12105b176e6 100644
--- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
+++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
@@ -39,6 +39,7 @@ import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.pinot.segment.local.PinotBuffersAfterClassCheckRule;
+import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
 import org.apache.pinot.segment.local.io.util.PinotDataBitSet;
 import org.apache.pinot.segment.local.segment.creator.SegmentTestUtils;
 import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
@@ -46,9 +47,11 @@ import org.apache.pinot.segment.local.segment.index.converter.SegmentV1V2ToV3For
 import org.apache.pinot.segment.local.segment.index.loader.columnminmaxvalue.ColumnMinMaxValueGeneratorMode;
 import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
 import org.apache.pinot.segment.local.segment.store.SegmentLocalFSDirectory;
+import org.apache.pinot.segment.local.startree.StarTreeBuilderUtils;
 import org.apache.pinot.segment.local.utils.SegmentOperationsThrottler;
 import org.apache.pinot.segment.local.utils.SegmentOperationsThrottlerSet;
 import org.apache.pinot.segment.spi.ColumnMetadata;
+import org.apache.pinot.segment.spi.ImmutableSegment;
 import org.apache.pinot.segment.spi.V1Constants;
 import org.apache.pinot.segment.spi.compression.ChunkCompressionType;
 import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
@@ -61,6 +64,7 @@ import org.apache.pinot.segment.spi.index.StandardIndexes;
 import org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl;
 import org.apache.pinot.segment.spi.index.reader.ForwardIndexReader;
 import org.apache.pinot.segment.spi.index.startree.AggregationFunctionColumnPair;
+import org.apache.pinot.segment.spi.index.startree.StarTreeV2;
 import org.apache.pinot.segment.spi.index.startree.StarTreeV2Metadata;
 import org.apache.pinot.segment.spi.store.SegmentDirectory;
 import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
@@ -81,6 +85,7 @@ import org.apache.pinot.spi.data.FieldSpec.DataType;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.data.readers.GenericRow;
 import org.apache.pinot.spi.utils.ByteArray;
+import org.apache.pinot.spi.utils.JsonUtils;
 import org.apache.pinot.spi.utils.ReadMode;
 import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
 import org.testng.annotations.AfterMethod;
@@ -2025,6 +2030,273 @@ public class SegmentPreProcessorTest implements PinotBuffersAfterClassCheckRule
     }
   }
 
+  /// A star-tree dimension column that is moved to 'noDictionaryColumns' without the star-tree being rebuilt leaves
+  /// the star-tree unreadable: its dimension forward index stores dictionary ids in a fixed-bit encoding whose width
+  /// is read from the main column metadata, which is now raw. The stale star-tree must be dropped so the segment
+  /// stays loadable, even when dynamic star-tree creation is disabled.
+  @Test
+  public void testStarTreeDimensionConvertedToNoDictionary()
+      throws Exception {
+    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+    Schema schema = new Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+        .addMetric("longCol", DataType.LONG)
+        .build();
+    IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+    indexingConfig.setStarTreeIndexConfigs(
+        List.of(new StarTreeIndexConfig(List.of("stringCol"), null, List.of("SUM__longCol"), null, 1000)));
+    buildStarTreeTestSegment(tableConfig, schema);
+
+    // Drift the config: the star-tree dimension is moved to noDictionaryColumns and the star-tree config is dropped,
+    // while dynamic star-tree creation stays disabled.
+    indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+    indexingConfig.setStarTreeIndexConfigs(null);
+    indexingConfig.setEnableDynamicStarTreeCreation(false);
+    IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+        SegmentPreProcessor processor = new SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+      assertTrue(processor.needProcess());
+      processor.process(SEGMENT_OPERATIONS_THROTTLER);
+    }
+    assertSegmentLoadsWithoutStarTree(indexLoadingConfig);
+
+    // The stale star-tree is gone, so there is nothing left to process
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+        SegmentPreProcessor processor = new SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+      assertFalse(processor.needProcess());
+    }
+  }
+
+  /// Same drift as [#testStarTreeDimensionConvertedToNoDictionary()], but the dict-to-raw conversion has already been
+  /// persisted by an earlier pre-processing round, so the segment on disk is already inconsistent and nothing else
+  /// needs updating. Pre-processing must still detect and repair it.
+  @Test
+  public void testStarTreeDimensionAlreadyConvertedToNoDictionary()
+      throws Exception {
+    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+    Schema schema = new Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+        .addMetric("longCol", DataType.LONG)
+        .build();
+    IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+    indexingConfig.setStarTreeIndexConfigs(
+        List.of(new StarTreeIndexConfig(List.of("stringCol"), null, List.of("SUM__longCol"), null, 1000)));
+    buildStarTreeTestSegment(tableConfig, schema);
+
+    // Convert the dimension column to raw while leaving the star-tree in place, reproducing the state an earlier
+    // pre-processing round leaves behind.
+    indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+    IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      new ForwardIndexHandler(segmentDirectory, indexLoadingConfig).updateIndices(segmentDirectory.createWriter());
+    }
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      assertFalse(segmentDirectory.getSegmentMetadata().getColumnMetadataFor("stringCol").hasDictionary());
+      assertNotNull(segmentDirectory.getSegmentMetadata().getStarTreeV2MetadataList());
+    }
+
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+        SegmentPreProcessor processor = new SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+      assertTrue(processor.needProcess());
+      processor.process(SEGMENT_OPERATIONS_THROTTLER);
+    }
+    assertSegmentLoadsWithoutStarTree(indexLoadingConfig);
+  }
+
+  /// The loader must not fail the whole segment over a stale star-tree even when pre-processing never gets a chance to
+  /// repair it, e.g. because it is skipped for the table.
+  @Test
+  public void testStarTreeDimensionConvertedToNoDictionaryWithoutPreprocess()
+      throws Exception {
+    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+    Schema schema = new Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+        .addMetric("longCol", DataType.LONG)
+        .build();
+    IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+    indexingConfig.setStarTreeIndexConfigs(
+        List.of(new StarTreeIndexConfig(List.of("stringCol"), null, List.of("SUM__longCol"), null, 1000)));
+    buildStarTreeTestSegment(tableConfig, schema);
+
+    indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+    IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      new ForwardIndexHandler(segmentDirectory, indexLoadingConfig).updateIndices(segmentDirectory.createWriter());
+    }
+
+    // Pre-processing would repair the segment, so it has to be off for the loader to ever see the stale star-tree.
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      assertTrue(ImmutableSegmentLoader.needPreprocess(segmentDirectory, indexLoadingConfig));
+    }
+
+    // 'skipSegmentPreprocess' is the knob that turns it off, and it takes effect through
+    // ImmutableSegmentLoader#needPreprocess, not SegmentPreProcessor#needProcess. IndexLoadingConfig snapshots it,
+    // so it needs a fresh config rather than an in-place edit of the table config.
+    indexingConfig.setSkipSegmentPreprocess(true);
+    IndexLoadingConfig skipPreprocessLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      assertFalse(ImmutableSegmentLoader.needPreprocess(segmentDirectory, skipPreprocessLoadingConfig));
+    }
+
+    // Ask the loader to pre-process, as the server does: it gates on needPreprocess() itself, so the flag keeps the
+    // stale star-tree in place and the star-tree loader has to cope with it rather than fail the load.
+    ImmutableSegment segment = ImmutableSegmentLoader.load(INDEX_DIR, skipPreprocessLoadingConfig, true);
+    try {
+      assertEquals(segment.getSegmentMetadata().getTotalDocs(), 5);
+      // The stale star-tree is still on disk; it is skipped at load time, not removed
+      assertNotNull(segment.getSegmentMetadata().getStarTreeV2MetadataList());
+      List<StarTreeV2> starTrees = segment.getStarTrees();
+      assertNotNull(starTrees);
+      assertTrue(starTrees.isEmpty());
+    } finally {
+      segment.destroy();
+    }
+  }
+
+  /// With dynamic star-tree creation enabled, the stale star-tree is not just dropped but rebuilt from the current
+  /// config, which no longer splits on the re-encoded column.
+  @Test
+  public void testStarTreeDimensionConvertedToNoDictionaryWithDynamicCreation()
+      throws Exception {
+    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+    Schema schema = new Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+        .addSingleValueDimension("intCol", DataType.INT)
+        .addMetric("longCol", DataType.LONG)
+        .build();
+    IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+    indexingConfig.setStarTreeIndexConfigs(
+        List.of(new StarTreeIndexConfig(List.of("stringCol", "intCol"), null, List.of("SUM__longCol"), null, 1000)));
+    buildStarTreeTestSegment(tableConfig, schema);
+
+    // 'stringCol' becomes raw and drops out of the split order, and the star-tree is rebuilt on 'intCol' alone
+    indexingConfig.setNoDictionaryColumns(List.of("stringCol"));
+    indexingConfig.setStarTreeIndexConfigs(
+        List.of(new StarTreeIndexConfig(List.of("intCol"), null, List.of("SUM__longCol"), null, 1000)));
+    indexingConfig.setEnableDynamicStarTreeCreation(true);
+    IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+        SegmentPreProcessor processor = new SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+      assertTrue(processor.needProcess());
+      processor.process(SEGMENT_OPERATIONS_THROTTLER);
+    }
+
+    ImmutableSegment segment = ImmutableSegmentLoader.load(INDEX_DIR, indexLoadingConfig, false);
+    try {
+      List<StarTreeV2> starTrees = segment.getStarTrees();
+      assertNotNull(starTrees);
+      assertEquals(starTrees.size(), 1);
+      assertEquals(starTrees.get(0).getMetadata().getDimensionsSplitOrder(), List.of("intCol"));
+    } finally {
+      segment.destroy();
+    }
+  }
+
+  /// Apache Pinot PR #19153 added star-tree support for dimensions stored as a `RAW` forward index with a separated
+  /// dictionary. Such a column still has a dictionary, so its star-tree stays readable and must NOT be treated as
+  /// stale: pre-processing has to flip the forward index to raw, keep the dictionary, and leave the star-tree alone.
+  @Test
+  public void testStarTreeDimensionConvertedToRawWithSeparatedDictionary()
+      throws Exception {
+    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName("testTable").build();
+    Schema schema = new Schema.SchemaBuilder().addSingleValueDimension("stringCol", DataType.STRING)
+        .addMetric("longCol", DataType.LONG)
+        .build();
+    IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+    indexingConfig.setStarTreeIndexConfigs(
+        List.of(new StarTreeIndexConfig(List.of("stringCol"), null, List.of("SUM__longCol"), null, 1000)));
+    buildStarTreeTestSegment(tableConfig, schema);
+
+    // Keep the star-tree config, but store the dimension as RAW forward index with the dictionary kept alongside
+    ObjectNode indexes = JsonUtils.newObjectNode();
+    ObjectNode forwardConfig = JsonUtils.newObjectNode();
+    forwardConfig.put("encodingType", "RAW");
+    indexes.set("forward", forwardConfig);
+    ObjectNode dictionaryConfig = JsonUtils.newObjectNode();
+    dictionaryConfig.put("disabled", false);
+    indexes.set("dictionary", dictionaryConfig);
+    tableConfig.setFieldConfigList(List.of(
+        new FieldConfig.Builder("stringCol").withEncodingType(FieldConfig.EncodingType.RAW)
+            .withIndexes(indexes)
+            .build()));
+    indexingConfig.setEnableDynamicStarTreeCreation(false);
+    IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
+
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+        SegmentPreProcessor processor = new SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+      // The forward index still has to be flipped to RAW, so there is work to do
+      assertTrue(processor.needProcess());
+      processor.process(SEGMENT_OPERATIONS_THROTTLER);
+    }
+
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      SegmentMetadataImpl segmentMetadata = segmentDirectory.getSegmentMetadata();
+      ColumnMetadata columnMetadata = segmentMetadata.getColumnMetadataFor("stringCol");
+      assertEquals(columnMetadata.getForwardIndexEncoding(), FieldConfig.EncodingType.RAW);
+      assertTrue(columnMetadata.hasDictionary());
+      // The star-tree is still loadable, so it must be left in place
+      assertNotNull(segmentMetadata.getStarTreeV2MetadataList());
+      assertTrue(StarTreeBuilderUtils.findUnloadableDimensions(segmentMetadata.getStarTreeV2MetadataList(),
+          segmentMetadata).isEmpty());
+    }
+
+    // Pre-processing must be idempotent here: neither the star-tree nor the forward index and dictionary handlers
+    // may ask for more work on a second round.
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+        SegmentPreProcessor processor = new SegmentPreProcessor(segmentDirectory, indexLoadingConfig)) {
+      assertFalse(processor.needProcess());
+    }
+
+    ImmutableSegment segment = ImmutableSegmentLoader.load(INDEX_DIR, indexLoadingConfig, false);
+    try {
+      List<StarTreeV2> starTrees = segment.getStarTrees();
+      assertNotNull(starTrees);
+      assertEquals(starTrees.size(), 1);
+      assertEquals(starTrees.get(0).getMetadata().getDimensionsSplitOrder(), List.of("stringCol"));
+      assertNotNull(segment.getDataSource("stringCol").getDictionary());
+    } finally {
+      segment.destroy();
+    }
+  }
+
+  private void buildStarTreeTestSegment(TableConfig tableConfig, Schema schema)
+      throws Exception {
+    FileUtils.deleteQuietly(TEMP_DIR);
+    SegmentGeneratorConfig config = new SegmentGeneratorConfig(tableConfig, schema);
+    config.setInstanceType(InstanceType.SERVER);
+    config.setOutDir(TEMP_DIR.getAbsolutePath());
+    config.setSegmentName(SEGMENT_NAME);
+
+    String[] stringValues = {"A", "C", "B", "C", "D"};
+    long[] longValues = {2, 1, 2, 3, 4};
+    List<GenericRow> rows = new ArrayList<>(stringValues.length);
+    for (int i = 0; i < stringValues.length; i++) {
+      GenericRow row = new GenericRow();
+      row.putValue("stringCol", stringValues[i]);
+      row.putValue("intCol", i % 3);
+      row.putValue("longCol", longValues[i]);
+      rows.add(row);
+    }
+
+    SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
+    driver.init(config, new GenericRowRecordReader(rows));
+    driver.build();
+
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      assertNotNull(segmentDirectory.getSegmentMetadata().getStarTreeV2MetadataList());
+    }
+  }
+
+  private void assertSegmentLoadsWithoutStarTree(IndexLoadingConfig indexLoadingConfig)
+      throws Exception {
+    try (SegmentDirectory segmentDirectory = new SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap)) {
+      assertNull(segmentDirectory.getSegmentMetadata().getStarTreeV2MetadataList());
+    }
+    ImmutableSegment segment = ImmutableSegmentLoader.load(INDEX_DIR, indexLoadingConfig, false);
+    try {
+      assertEquals(segment.getSegmentMetadata().getTotalDocs(), 5);
+      assertTrue(segment.getStarTrees() == null || segment.getStarTrees().isEmpty());
+    } finally {
+      segment.destroy();
+    }
+  }
+
   @Test
   public void testStarTreeCreationWithInvalidFunctionColumnPair()
       throws Exception {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org