feat: Extend FilterSegmentPruner to prune on ClusterGroupTuples by cecemei · Pull Request #20149 · apache/druid

cecemei · 2026-08-25T19:02:33Z

Description

FilterSegmentPruner currently only prunes segments using shard-spec domain ranges (ShardSpec#possibleInDomain). This adds a second, independent check against DataSegment#getClusterGroups(). When a segment carries ClusterGroupTuples, the pruner now also excludes the segment if none of its tuples can satisfy the filter.

This PR has:

gianm · 2026-08-26T03:28:01Z

+    final VirtualColumns.Node domainNode = domainVirtualColumns.getNode(column);
+    if (domainNode != null) {
+      final VirtualColumn queryEquivalent = getQueryEquivalent(domainNode);
+      if (queryEquivalent != null && filterFields.contains(queryEquivalent.getOutputName())) {


I think that cluster group virtual columns end up being directly queryable. If this is right then the matching should apply to both the virtual column and to the materialized column (i.e. WHERE group_key = 'something', where group_key is the name of the materialized cluster column.)

Do you mean that we should match virtual column for both and never match a virtual column (either from clustering group or query) with a physical column? I just added that check, also with test testPruneClusterGroupTuplesVirtualColumnNoQueryVirtualColumnNeverPrunes and testPruneClusterGroupTuplesShadowedByQueryVirtualColumnNeverPrunes

I meant that when a cluster group tuple has a part defined by a virtual column, we should be able to prune on that part if it's referenced either using a query-time virtual column, or referenced directly. To me it looks like the current code only works if it's referenced through a query-time virtual column.

The scenario I'm talking about is like:

cluster group key is (vdim1) defined as concat(dim1, 'foo')

we have a segment using that cluster group config, with cluster groups ('abcfoo') and ('deffoo')

query WHERE CONCAT(dim1, 'foo') = 'barfoo' should prune out this segment

query WHERE vdim1 = 'barfoo' should also prune out this segment

gianm · 2026-08-26T03:37:32Z

+
+    // same expression, same name
+    VirtualColumns queryVirtualColumns = VirtualColumns.create(
+        new ExpressionVirtualColumn("vdim1", "concat(dim1, 'foo')", ColumnType.STRING, TestExprMacroTable.INSTANCE)


Please add a test that confirms vdim1 set to a different expression never prunes.

Please also add a test for querying vdim1 directly when there is no virtual column with that name. (It should prune when cluster groups are present on the segment, because in that case vdim1 is materialized.)

we have testPruneClusterGroupTuples for testing querying directly.

FrankChen021

Severity	Findings
P0	0
P1	1
P2	0
P3	0
Total	1

Severity	Findings
P0	0
P1	1
P2	0
P3	0
Total	1

Reviewed 2 of 2 changed files.

Validation: focused git diff --check passed. Builds and tests were not run.

This is an automated review by Codex GPT-5.6-Luna(max)

FrankChen021 · 2026-08-26T12:27:14Z

+        );
+        optFilterRangeSet.ifPresent(rangeSet -> filterDomain.put(column, rangeSet));
+      }
+    } else if (filterFields.contains(column)) {


[P1] Ignores query-VC shadowing of physical cluster columns

When a query virtual column has the same name as a physical clustering column but a different expression, query VCs shadow physical columns. This branch still applies the filter range to the physical tuple value, so a segment can be pruned even though the virtual expression matches rows inside it. Resolve query virtual columns first and only use direct tuple matching when no query VC shadows the name; add a regression test.

updated to make sure never prunes when query VC shadows physical column

Confirmed the prior same-name physical-column shadowing finding is resolved by the current guard and regression test. A separate P1 remains at processing/src/main/java/org/apache/druid/query/filter/FilterSegmentPruner.java:220: query virtual-column dependencies can shadow physical inputs, causing incorrect cluster-group pruning. Reviewed 2 of 2 changed files.

FrankChen021

Severity	Findings
P0	0
P1	1
P2	0
P3	0
Total	1

Reviewed 2 of 2 changed files. The prior same-name physical-column shadowing concern is resolved by the current guard and regression test; this review found a separate dependency-shadowing correctness issue.

Validation: focused git diff --check passed. Builds and tests were not run.

This is an automated review by Codex GPT-5.6-Luna(max)

FrankChen021 · 2026-08-27T12:32:26Z

+  {
+    final VirtualColumns.Node domainNode = domainVirtualColumns.getNode(column);
+    if (domainNode != null) {
+      final VirtualColumn queryEquivalent = getQueryEquivalent(domainNode);


P1 Avoid shadowed query dependencies in VC equivalence

getQueryEquivalent(domainNode) matches a segment-side virtual column without accounting for query virtual-column dependencies. For example, with segment v0 = dim1 and query VCs dim1 = dim2 plus q = dim1, q can be treated as equivalent to v0 even though it reads dim2. The pruner can then use a range for q against the wrong cluster-group value and prune segments that contain matching rows. Resolve equivalence from the query dependency graph, and add a regression test for this shadowed-dependency case.

FrankChen021

Severity	Findings
P0	0
P1	1
P2	0
P3	0
Total	1

The new virtual-column dependency accounting can conflate a query virtual column with a physical column that shadows one of its dependencies. For example, with physical a=x and v0=concat(a,b), and query virtual columns b=x and q=concat(b,b), the rewritten expressions can look identical even though query b shadows physical b. This can cause cluster-group pruning to discard segments that still contain matching rows. Compare resolved bindings or reject these rewrite collisions, and add a nested-shadow regression test.

Reviewed 3 of 3 changed files.

Validation: focused git diff --no-ext-diff --check passed. Builds and tests were not run.

This is an automated review by Codex GPT-5.6-Luna(max)

FrankChen021 · 2026-08-29T13:17:13Z

+    if (matched != null &&
+        // guardrail check for expression collision when a virtual column shadows the physical column
+        // e.x. otherNode v0 = dim1 and VCs dim1 = dim2 plus q = dim1, q can be treated as equivalent to v0 even though it reads physical column dim2
+        getNode(matched.getOutputName()).getDependencies().size() == otherNode.getDependencies().size()) {


P1 Virtual-column dependency counting misses physical-name collisions

The dependency count can conflate a query virtual column with a physical column that shadows one of its dependencies. For example, with physical a=x and v0=concat(a,b), and query virtual columns b=x and q=concat(b,b), the rewritten expressions can look identical even though query b shadows physical b. The cluster-group pruner can then treat q as v0 and discard segments that contain matching rows. Compare resolved bindings or reject rewrite collisions, and add a nested-shadow regression test.

pruning

9e18e17

cecemei changed the title ~~pruning~~ feat: Extend FilterSegmentPruner to prune on ClusterGroupTuples Aug 25, 2026

pruning2

58f8bcc

cecemei marked this pull request as ready for review August 25, 2026 19:14

gianm reviewed Aug 26, 2026

View reviewed changes

FrankChen021 reviewed Aug 26, 2026

View reviewed changes

pruning3

19c1a92

FrankChen021 reviewed Aug 27, 2026

View reviewed changes

transitive shadow

badbeae

github-actions Bot added the Area - Segment Format and Ser/De label Aug 28, 2026

direct-query

19fa4ec

FrankChen021 reviewed Aug 29, 2026

View reviewed changes

Uh oh!

Conversation

cecemei commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FrankChen021 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!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FrankChen021 left a comment

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

cecemei commented Aug 25, 2026 •

edited

Loading