refactor: adds StringColumnFormatSpec for string dimension configs by jaykanakiya · Pull Request #19258 · apache/druid

jaykanakiya · 2026-04-02T18:40:39Z

Description

Adds StringColumnFormatSpec to indexSpec, allowing string column format settings to be configured at the job level with per-column overrides via dimensionSchema.columnFormatSpec. Follows similar pattern as #17762

Release note

Key changed/added classes in this PR

StringColumnFormatSpec
StringDimensionSchema
IndexSpec
Sink

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
a release note entry in the PR description.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added or updated version, license, or notice information in licenses.yaml
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
added integration tests.
been tested in a test Druid cluster.

clintropolis · 2026-04-02T19:08:38Z

processing/src/main/java/org/apache/druid/data/input/impl/StringDimensionSchema.java

      @JsonProperty("multiValueHandling") MultiValueHandling multiValueHandling,
      @JsonProperty("createBitmapIndex") Boolean createBitmapIndex,
-      @JsonProperty("maxStringLength") @Nullable Integer maxStringLength
+      @JsonProperty("maxStringLength") @Nullable Integer maxStringLength,


since maxStringLength hasn't been released, should we just remove this in favor of the columnFormatSpec?

Removed maxStringLength from StringDimensionSchema.

clintropolis · 2026-04-02T19:28:28Z

processing/src/main/java/org/apache/druid/segment/StringColumnFormatSpec.java

+
+  @JsonCreator
+  public StringColumnFormatSpec(
+      @JsonProperty("createBitmapIndex") @Nullable Boolean createBitmapIndex,


I was thinking we would switch this to using something like BitmapIndexType we have for NestedCommonFormatColumnFormatSpec used to control which indexes we build for long and double json fields, since ideally we can introduce some flexibility here so that we can more easily support different kinds of indexes on string columns that specifying with a bunch of booleans would be tedious.

I think this implementation can be a lot simpler than BitmapIndexType for now, we can worry about how index building can be abstracted for strings in the dimension indexer/mergers/serializers later when we actually introduce such a thing. I think there are 2 options we need to add to model the current state, dictionaryEncodedValueIndex and none, so we could make an abstract StringIndexType or StringBitmapIndexType and have basically empty implementations for NoIndex and DictionaryEncodedValueIndex (only needs like equals/hashcode implemented i think and maybe a static final instantiation to share) to use as a placeholder for future enhancement.

Then we can wire this indexType with those values up to the existing boolean all the stuff uses.

Can you look into making this change?

Added StringBitmapIndexType class with dictionaryEncodedValueIndex and none options and replaced createBitmapIndex in columnFormatSpec.

clintropolis · 2026-04-02T19:37:12Z

server/src/main/java/org/apache/druid/segment/realtime/sink/Sink.java


  private FireHydrant makeNewCurrIndex(long minTimestamp, DataSchema schema)
  {
+    final DimensionsSpec dimensionsSpec = resolveEffectiveDimensionsSpec(schema.getDimensionsSpec());


ah, i guess we have to do this here now because we need to know stuff up front when making the column indexers, seems worth leaving a comment explaining why we do this

Added a comment.

clintropolis · 2026-04-02T19:40:28Z

server/src/main/java/org/apache/druid/segment/realtime/sink/Sink.java

+  private DimensionsSpec resolveEffectiveDimensionsSpec(DimensionsSpec dimensionsSpec)
+  {
+    if (indexSpec == null) {
+      return dimensionsSpec;
+    }
+    final List<DimensionSchema> effectiveDimensions = dimensionsSpec.getDimensions()
+        .stream()
+        .map(dim -> dim.getEffectiveSchema(indexSpec))
+        .collect(Collectors.toList());
+    return DimensionsSpec.builder(dimensionsSpec)
+                         .setDimensions(effectiveDimensions)
+                         .build();
+  }
+


i think in the constructor we should call getEffectiveSpec on the indexSpec so that it is filled in with system defaults. We should also probably set it to IndexSpec.default() if it is null, so then we always resolve the effective schemas to fill in system default values

Added IndexSpec.getDefault() in the constructor as the default.

aho135 · 2026-04-09T04:02:08Z

Thanks for the change @jaykanakiya!

Maybe we could do this as a follow up, but it looks like the indexSpec documentation is pretty outdated. It'd be helpful to see an example for stringColumnFormatSpec

aho135 · 2026-04-09T04:21:59Z

processing/src/main/java/org/apache/druid/segment/IndexSpec.java

+          StringColumnFormatSpec.getEffectiveFormatSpec(stringColumnFormatSpec, this)
+      );
+    } else if (defaultSpec.stringColumnFormatSpec != null) {
+      bob.withStringColumnFormatSpec(


Is this needed? stringColumnFormatSpec is always null in the defaultSpec

I see a similar pattern for other properties but I don't think it's actually needed in this case

defautlSpec can be set in runtime.properties to set system level default overrides, so we want it to be like this i think

aho135 · 2026-04-09T04:29:28Z

processing/src/main/java/org/apache/druid/segment/StringDimensionHandler.java

  )
  {
-    this(dimensionName, multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, StringDimensionSchema.getDefaultMaxStringLength());
+    this(dimensionName, multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, null);


Suggested change

this(dimensionName, multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, null);

this(dimensionName, multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, BuiltInTypesModule.getMaxStringLength());

Or alternatively make BuiltInTypesModule.MAX_STRING_LENGTH public

aho135 · 2026-04-09T04:31:25Z

processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java

  )
  {
-    this(multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, StringDimensionSchema.getDefaultMaxStringLength());
+    this(multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, null);


Can also use BuiltInTypesModule for readability here

Updated to use BuiltInTypesModule.

aho135 · 2026-04-09T04:40:57Z

processing/src/main/java/org/apache/druid/data/input/impl/StringDimensionSchema.java

  @Override
  public DimensionHandler getDimensionHandler()
  {
+    Integer maxStringLength = columnFormatSpec != null ? columnFormatSpec.getMaxStringLength() : null;


Leaving a comment so I can revisit this tomorrow. Why do we only pass in maxStringLength here, but use parent defaults for createBitmapIndex/multiValueHandling?

yea, I think we should get all of the things from the formatSpec if they are defined there, and fallback to the parent if not set

Fixed this.

aho135 · 2026-04-09T19:35:58Z

processing/src/test/java/org/apache/druid/data/input/impl/StringDimensionSchemaTest.java

+                        + "  \"multiValueHandling\" : \"SORTED_SET\",\n"
+                        + "  \"createBitmapIndex\" : false,\n"
+                        + "  \"columnFormatSpec\" : { \"maxStringLength\" : 200 }\n"
+                        + "}";


It would be good to have a few test cases with different variations of columnFormatSpec. For example, tests where multiValueHandling is also present in columnFormatSpec to document the order of precedence

Add tests to verify precedence.

clintropolis

I think this mostly lgtm now, thanks for making this change 👍

I actually think there is 1 more change that we want to do, but I think its also ok to do as a follow-up.

The change is that I want to switch string columns to actually store the StringColumnFormatSpec in the json of the DictionaryEncodedColumnPartSerde, and switch string column stuff to implementing ColumnFormat like the nested/auto column does instead of using the fallback CapabilitiesBasedFormat. That way, it can recreate a handler with the same parameters that were used to write it for things like compaction (by using the stored formatspec to create the format which is what drives what parameters are set on the indexer/merger/serializer stuff).

clintropolis · 2026-04-10T18:12:58Z

processing/src/main/java/org/apache/druid/segment/IndexSpec.java

+          StringColumnFormatSpec.getEffectiveFormatSpec(stringColumnFormatSpec, this)
+      );
+    } else if (defaultSpec.stringColumnFormatSpec != null) {
+      bob.withStringColumnFormatSpec(


defautlSpec can be set in runtime.properties to set system level default overrides, so we want it to be like this i think

clintropolis · 2026-04-10T18:42:10Z

processing/src/main/java/org/apache/druid/data/input/impl/StringDimensionSchema.java

  @Override
  public DimensionHandler getDimensionHandler()
  {
+    Integer maxStringLength = columnFormatSpec != null ? columnFormatSpec.getMaxStringLength() : null;


yea, I think we should get all of the things from the formatSpec if they are defined there, and fallback to the parent if not set

aho135 · 2026-04-10T22:59:38Z

processing/src/main/java/org/apache/druid/data/input/impl/StringDimensionSchema.java

+    MultiValueHandling mvh = columnFormatSpec != null && columnFormatSpec.getMultiValueHandling() != null
+        ? columnFormatSpec.getMultiValueHandling()
+        : getMultiValueHandling();
+    boolean bitmap = columnFormatSpec != null && columnFormatSpec.getIndexType() != null


Nit: we can do a small refactor to only check if columnFormatSpec is null one time

Updated to null check columnFormatSpec once

adds StringColumnFormatSpec for string configs

24a0826

github-actions bot added the Area - Segment Format and Ser/De label Apr 2, 2026

clintropolis requested changes Apr 2, 2026

View reviewed changes

jaykanakiya added 2 commits April 3, 2026 09:28

fix tests

e658c7b

address PR comments and fix tests

2f4badc

jaykanakiya requested a review from clintropolis April 6, 2026 15:55

aho135 reviewed Apr 9, 2026

View reviewed changes

clintropolis added this to the 37.0.0 milestone Apr 9, 2026

clintropolis reviewed Apr 10, 2026

View reviewed changes

jaykanakiya added 2 commits April 10, 2026 13:29

address review comments + add tests

450a968

Merge branch 'master' into string-column-format-spec

ca5738c

aho135 reviewed Apr 10, 2026

View reviewed changes

null check columnFormatSpec only once

96b22dc

	this(dimensionName, multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, null);
	this(dimensionName, multiValueHandling, hasBitmapIndexes, hasSpatialIndexes, BuiltInTypesModule.getMaxStringLength());

Conversation

jaykanakiya commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release note

Key changed/added classes in this PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aho135 commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aho135 Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clintropolis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jaykanakiya commented Apr 2, 2026 •

edited

Loading

aho135 commented Apr 9, 2026 •

edited

Loading

aho135 Apr 9, 2026 •

edited

Loading