compaction now only identifies multi-value dimensions for dimension schemas which can possibly produce them (#18760)

clintropolis · web-flow · commit 9b6d18fe511c · 2025-12-02T15:25:23.000-08:00
changes:
* added `DimensionSchema.canBeMultiValued`, defaulting to false but true for `StringDimensionSchema` and `NewSpatialDimensionSchema`
* `CompactionTask.identifyMultiValuedDimensions` now checks `DimensionSchema.canBeMultiValued()` instead of checking for `ColumnType.STRING` from `DimensionSchema.getColumnType()`
* added test to confirm behavior using `AutoTypeColumnSchema` with a specified `castToType` of `ColumnType.STRING`, which prior to changes would identify as multi-valued despite auto never producing them
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
@@ -84,7 +84,6 @@
 import org.apache.druid.segment.Segment;
 import org.apache.druid.segment.column.ColumnCapabilities;
 import org.apache.druid.segment.column.ColumnHolder;
-import org.apache.druid.segment.column.ColumnType;
 import org.apache.druid.segment.incremental.AppendableIndexSpec;
 import org.apache.druid.segment.indexing.CombinedDataSchema;
 import org.apache.druid.segment.indexing.DataSchema;
@@ -466,13 +465,13 @@ public boolean isPerfectRollup()
    * Checks if multi-valued string dimensions need to be analyzed by downloading the segments.
    * This method returns true only for MSQ engine when either of the following holds true:
    * <ul>
-   * <li> Range partitioning is done on a string dimension or an unknown dimension
+   * <li> Range partitioning is done on a possibly multi-valued string dimension or an unknown dimension
    * (since MSQ does not support partitioning on a multi-valued string dimension) </li>
-   * <li> Rollup is done on a string dimension or an unknown dimension
+   * <li> Rollup is done on a multi-valued string dimension or an unknown dimension
    * (since MSQ requires multi-valued string dimensions to be converted to arrays for rollup) </li>
    * </ul>
-   * @return false for native engine, true for MSQ engine only when partitioning or rollup is done on a string
-   * or unknown dimension.
+   * @return false for native engine, true for MSQ engine only when partitioning or rollup is done on a multi-valued
+   * string or unknown dimension.
    */
   boolean identifyMultiValuedDimensions()
   {
@@ -481,28 +480,31 @@ boolean identifyMultiValuedDimensions()
     }
     // Rollup can be true even when granularitySpec is not known since rollup is then decided based on segment analysis
     final boolean isPossiblyRollup = granularitySpec == null || !Boolean.FALSE.equals(granularitySpec.isRollup());
-    boolean isRangePartitioned = tuningConfig != null
-                                 && tuningConfig.getPartitionsSpec() instanceof DimensionRangePartitionsSpec;
+    final DimensionRangePartitionsSpec rangeSpec;
+    if (tuningConfig != null && tuningConfig.getPartitionsSpec() instanceof DimensionRangePartitionsSpec) {
+      rangeSpec = (DimensionRangePartitionsSpec) tuningConfig.getPartitionsSpec();
+    } else {
+      rangeSpec = null;
+    }
+    final boolean isRangePartitioned = rangeSpec != null;
 
     if (dimensionsSpec == null || dimensionsSpec.getDimensions().isEmpty()) {
       return isPossiblyRollup || isRangePartitioned;
     } else {
-      boolean isRollupOnStringDimension = isPossiblyRollup &&
-                                          dimensionsSpec.getDimensions()
-                                                        .stream()
-                                                        .anyMatch(dim -> ColumnType.STRING.equals(dim.getColumnType()));
+      boolean isRollupOnMultiValueStringDimension = isPossiblyRollup &&
+                                                    dimensionsSpec.getDimensions()
+                                                                  .stream()
+                                                                  .anyMatch(DimensionSchema::canBeMultiValued);
 
-      boolean isPartitionedOnStringDimension =
+      boolean isPartitionedOnMultiValueStringDimension =
           isRangePartitioned &&
           dimensionsSpec.getDimensions()
                         .stream()
                         .anyMatch(
-                            dim -> ColumnType.STRING.equals(dim.getColumnType())
-                                   && ((DimensionRangePartitionsSpec) tuningConfig.getPartitionsSpec())
-                                       .getPartitionDimensions()
-                                       .contains(dim.getName())
+                            dim -> dim.canBeMultiValued()
+                                   && rangeSpec.getPartitionDimensions().contains(dim.getName())
                         );
-      return isRollupOnStringDimension || isPartitionedOnStringDimension;
+      return isRollupOnMultiValueStringDimension || isPartitionedOnMultiValueStringDimension;
     }
   }
 
diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/CompactionTaskTest.java
@@ -97,6 +97,7 @@
 import org.apache.druid.query.aggregation.firstlast.last.DoubleLastAggregatorFactory;
 import org.apache.druid.query.expression.TestExprMacroTable;
 import org.apache.druid.query.filter.SelectorDimFilter;
+import org.apache.druid.segment.AutoTypeColumnSchema;
 import org.apache.druid.segment.IndexIO;
 import org.apache.druid.segment.IndexMergerV9;
 import org.apache.druid.segment.IndexSpec;
@@ -1674,6 +1675,33 @@ public void testMSQRangePartitionOnStringNeedsMVDInfo()
     Assert.assertTrue(compactionTask.identifyMultiValuedDimensions());
   }
 
+  @Test
+  public void testMSQRangePartitionOnAutoStringDoesNotNeedMVDInfo()
+  {
+    final Builder builder = new Builder(
+        DATA_SOURCE,
+        segmentCacheManagerFactory
+    );
+    builder.inputSpec(new CompactionIntervalSpec(COMPACTION_INTERVAL, SegmentUtils.hashIds(SEGMENTS)));
+    builder.compactionRunner(new TestMSQCompactionRunner());
+
+    DimensionSchema stringDim = new AutoTypeColumnSchema("string_dim_1", ColumnType.STRING, null);
+    builder.tuningConfig(TuningConfigBuilder.forCompactionTask()
+                                            .withForceGuaranteedRollup(true)
+                                            .withPartitionsSpec(
+                                                new DimensionRangePartitionsSpec(
+                                                    3,
+                                                    null,
+                                                    List.of(stringDim.getName()),
+                                                    false
+                                                )
+                                            )
+                                            .build());
+    builder.dimensionsSpec(new DimensionsSpec(ImmutableList.of(stringDim)));
+    CompactionTask compactionTask = builder.build();
+    Assert.assertFalse(compactionTask.identifyMultiValuedDimensions());
+  }
+
   @Test
   public void testChooseFinestGranularityWithNulls()
   {
diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java b/processing/src/main/java/org/apache/druid/data/input/impl/DimensionSchema.java
@@ -176,6 +176,17 @@ public boolean hasBitmapIndex()
   @JsonIgnore
   public abstract ColumnType getColumnType();
 
+  /**
+   * Returns true if the {@link DimensionHandler#makeIndexer()} of this schema can produce multi-valued
+   * {@link ColumnType#STRING} columns. This method is used by MSQ compaction to determine if it needs to download the
+   * segments to check if any are actually multi-valued.
+   */
+  @JsonIgnore
+  public boolean canBeMultiValued()
+  {
+    return false;
+  }
+
   @JsonIgnore
   public DimensionHandler getDimensionHandler()
   {
diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/NewSpatialDimensionSchema.java b/processing/src/main/java/org/apache/druid/data/input/impl/NewSpatialDimensionSchema.java
@@ -69,6 +69,12 @@ public ColumnType getColumnType()
     return ColumnType.STRING;
   }
 
+  @Override
+  public boolean canBeMultiValued()
+  {
+    return true;
+  }
+
   @Override
   public DimensionHandler getDimensionHandler()
   {
diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/StringDimensionSchema.java b/processing/src/main/java/org/apache/druid/data/input/impl/StringDimensionSchema.java
@@ -64,6 +64,12 @@ public ColumnType getColumnType()
     return ColumnType.STRING;
   }
 
+  @Override
+  public boolean canBeMultiValued()
+  {
+    return true;
+  }
+
   @Override
   public DimensionHandler getDimensionHandler()
   {

Original file line number	Diff line number	Diff line change
`@@ -69,6 +69,12 @@ public ColumnType getColumnType()`
`69`	`69`	`return ColumnType.STRING;`
`70`	`70`	`}`
`71`	`71`
	`72`	`+ @Override`
	`73`	`+ public boolean canBeMultiValued()`
	`74`	`+ {`
	`75`	`+ return true;`
	`76`	`+ }`
	`77`	`+`
`72`	`78`	`@Override`
`73`	`79`	`public DimensionHandler getDimensionHandler()`
`74`	`80`	`{`
Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,12 @@ public ColumnType getColumnType()`
`64`	`64`	`return ColumnType.STRING;`
`65`	`65`	`}`
`66`	`66`
	`67`	`+ @Override`
	`68`	`+ public boolean canBeMultiValued()`
	`69`	`+ {`
	`70`	`+ return true;`
	`71`	`+ }`
	`72`	`+`
`67`	`73`	`@Override`
`68`	`74`	`public DimensionHandler getDimensionHandler()`
`69`	`75`	`{`