Add alias-based dynamic filter assertions for plan matching by piotrrzysko · Pull Request #28334 · trinodb/trino

piotrrzysko · 2026-02-17T13:31:22Z

Description

Summary

This PR introduces a new approach for asserting dynamic filters in plan matcher tests. The new framework uses logical aliases to connect dynamic filter producers (at Join/SemiJoin nodes) with their consumers (at FilterNode).

Why?

The existing dynamic filter assertions lack explicit links between producers and consumers. Tests could only specify that a Join produces a dynamic filter and that somewhere in the left subtree there's a matching consumer—but couldn't explicitly assert which FilterNode consumes a given dynamic filter. This made it impossible to verify correct DF placement in plans with multiple FilterNodes or multiple dynamic filters.

Approach

Tests assign string aliases (e.g., "DF1") to dynamic filters. During bottom-up plan matching, consumer matchers at FilterNodes collect candidate DynamicFilterIds that match the expected pattern and associate them with the alias. As matching progresses up the tree, candidates are narrowed: multiple FilterNodes with the same alias intersect their candidates, and the producer matcher at Join/SemiJoin resolves to exactly one ID by verifying the build symbol.

API Change

Before

  join(RIGHT, builder -> builder
      .equiCriteria("ORDERS_OK", "LINEITEM_OK")
      .dynamicFilter(BIGINT, "ORDERS_OK", "LINEITEM_OK")
      .left(anyTree(tableScan("orders", ...)))
      .right(exchange(tableScan("lineitem", ...))))

After

  join(RIGHT, builder -> builder
      .equiCriteria("ORDERS_OK", "LINEITEM_OK")
      .addDynamicFilter("DF", "LINEITEM_OK")
      .left(
          filter(TRUE,
              df -> df.addConsumer(c -> c.alias("DF").expression(BIGINT, "ORDERS_OK")),
              tableScan("orders", ...)))
      .right(exchange(tableScan("lineitem", ...))))

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

Bundling parameters into a context object allows adding new fields without modifying all Matcher implementations.

...in/src/test/java/io/trino/sql/planner/optimizations/TestRemoveUnsupportedDynamicFilters.java

findepi · 2026-02-19T10:51:59Z

cc @sopel39

Copilot

Pull request overview

This PR updates Trino’s plan-matcher test framework to support alias-based dynamic filter assertions, allowing tests to explicitly link dynamic filter producers (Join/SemiJoin) to their consuming FilterNodes. It also refactors matcher APIs to pass a unified MatchContext, enabling dynamic-filter candidate propagation during bottom-up matching.

Changes:

Introduce dynamic-filter alias tracking (DynamicFilterAlias, DynamicFilterConsumerMatcher, DynamicFilterProducers, MatchingDynamicFilters) and enforce alias resolution in PlanAssert.
Refactor matcher plumbing to use MatchContext and thread dynamic-filter candidate sets through PlanMatchingVisitor/MatchResult.
Migrate existing plan tests and matchers to the new dynamic filter and matcher APIs.

Reviewed changes

Copilot reviewed 63 out of 63 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
core/trino-main/src/test/java/io/trino/sql/planner/optimizations/TestUnaliasSymbolReferences.java	Migrates dynamic filter assertions to alias-based producer/consumer patterns.
core/trino-main/src/test/java/io/trino/sql/planner/optimizations/TestRemoveUnsupportedDynamicFilters.java	Updates tests to assert no DF consumers via the new consumer matcher API.
core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestDetermineTableScanNodePartitioning.java	Updates custom matcher signature to use `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/WindowMatcher.java	Switches matcher to `MatchContext` and uses `context.symbolAliases()`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/ValuesMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/UnnestMatcher.java	Switches matcher to `MatchContext` and updates alias resolution usage.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TopNRankingMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TopNMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TableWriterMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TableScanMatcher.java	Switches matcher to `MatchContext` and routes metadata/session through context.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TableFunctionProcessorMatcher.java	Switches matcher to `MatchContext` and updates symbol alias mapping.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TableFunctionMatcher.java	Switches matcher to `MatchContext` and updates symbol alias mapping.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/TableExecuteMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/SymbolCardinalityMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/StatsOutputRowCountMatcher.java	Switches matcher to `MatchContext` and pulls stats via `context.stats()`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/SpatialJoinMatcher.java	Switches matcher to `MatchContext` and updates expression verification.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/SortMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/SemiJoinMatcher.java	Reworks semi-join DF matching to use alias-based candidate resolution via `MatchingDynamicFilters`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/SemiJoinDynamicFilterProducer.java	New helper type to express semi-join DF expectations (ignore / none / alias).
core/trino-main/src/test/java/io/trino/sql/planner/assertions/RowNumberMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/RemoteSourceMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/PredicateMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/PlanNodeMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/PlanMatchingVisitor.java	Threads dynamic-filter matching state through matching/alias propagation.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/PlanMatchPattern.java	Updates DF APIs (`filter`, `semiJoin`, join DF producers) and removes old DF pattern type.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/PlanAssert.java	Adds final assertion that all DF aliases resolve to a single unique ID.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/PatternRecognitionMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/OutputMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/OffsetMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/NotPlanNodeMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/MatchingDynamicFilters.java	New alias→candidate-id intersection/merge structure for DF matching.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/Matcher.java	Changes matcher API to accept a single `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/MatchResult.java	Adds `MatchingDynamicFilters` to match results and new helpers.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/MatchContext.java	New unified context object passed to matchers.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/MarkDistinctMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/LimitMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/JoinMatcher.java	Replaces old DF expression matching with alias-based producer resolution.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/IndexSourceMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/IndexJoinMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/IdentityProjectionMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/GroupIdMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/FilterMatcher.java	Makes filter matching ignore dynamic conjuncts by extracting them first.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/ExchangeMatcher.java	Switches matcher to `MatchContext` and updates alias updates.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterProducers.java	New join producer expectation container (alias→build symbol).
core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterConsumerMatcher.java	New filter-node consumer matcher that records DF candidates per alias.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterAlias.java	New value type for test-defined DF aliases.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/DistinctMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/DistinctLimitMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/CorrelationMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/CorrelatedJoinMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/ConnectorAwareTableScanMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/BaseStrictSymbolsMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/AliasMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/AggregationStepMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/AggregationMatcher.java	Switches matcher to `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/assertions/AdaptivePlanMatcher.java	Switches matcher to `MatchContext` and propagates it when visiting initial plans.
core/trino-main/src/test/java/io/trino/sql/planner/TestPredicatePushdownWithoutDynamicFilter.java	Updates semi-join DF expectations to `noDynamicFilter()`.
core/trino-main/src/test/java/io/trino/sql/planner/TestPredicatePushdown.java	Migrates join/semi-join DF assertions to alias-based producer/consumer patterns.
core/trino-main/src/test/java/io/trino/sql/planner/TestLogicalPlanner.java	Migrates DF assertions away from removed `DynamicFilterPattern`.
core/trino-main/src/test/java/io/trino/sql/planner/TestInsert.java	Updates custom matcher signature to use `MatchContext`.
core/trino-main/src/test/java/io/trino/sql/planner/TestDynamicFilter.java	Migrates many DF plan assertions to alias-based producer/consumer patterns.
core/trino-main/src/test/java/io/trino/sql/planner/TestAddDynamicFilterSource.java	Migrates DF assertions to alias-based producer/consumer patterns.
core/trino-main/src/test/java/io/trino/sql/planner/AbstractPredicatePushdownTest.java	Migrates semi-join DF assertions to alias-based producer/consumer patterns with enable/disable switching.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T11:29:04Z

core/trino-main/src/test/java/io/trino/sql/planner/assertions/MatchingDynamicFilters.java

+                Set<DynamicFilterId> mergedCandidates = entry.getValue().stream()
+                        .map(Candidates::candidates)
+                        .reduce((candidates1, candidates2) -> Sets.intersection(candidates1, candidates2).immutableCopy())
+                        .orElseThrow();
+                candidates.put(entry.getKey(), mergedCandidates);


MatchingDynamicFilters.Builder.build() can produce an empty mergedCandidates set when intersecting candidate sets for the same alias. That breaks Builder.add()'s non-empty invariant and can later make addAll()/merge() throw instead of returning NO_MATCH when aliases can't be reconciled across multiple FilterNodes. Consider treating an empty intersection as a match failure or ensuring build never emits empty candidate sets in a way that doesn't crash the matcher pipeline.

core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterConsumerMatcher.java

Praveen2112 · 2026-03-09T08:25:45Z

core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterConsumerMatcher.java

+    @Override
+    public boolean shapeMatches(PlanNode node)
+    {
+        return node instanceof FilterNode;


Apart from being a FilterNode - shouldn't we check it has predicate based on dynamic filters ?

It shouldn't hurt. Added.

Praveen2112 · 2026-03-09T13:59:32Z

core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterAlias.java

+
+import static java.util.Objects.requireNonNull;
+
+public record DynamicFilterAlias(String alias)


nit: Any specific reason for this abstraction instead of using String ? Alias is more of a name right ?

It’s more explicit.

We store aliases in maps, e.g., Map<DynamicFilterAlias, Set<DynamicFilterConsumer>>. I prefer this over Map<String, Set<DynamicFilterConsumer>>.

Also, we already have io.trino.sql.planner.assertions.SymbolAlias, which is similarly a thin wrapper over String.

Praveen2112 · 2026-03-09T14:04:09Z

core/trino-main/src/test/java/io/trino/sql/planner/assertions/MatchingDynamicFilters.java

+            if (entry.getValue().size() != 1) {
+                return true;
+            }


If an alias can have only a single entry, shouldn't we fail it a bit early ?

My reply above should make this clearer. We call this method at the end of plan matching to ensure a 1:1 mapping. We can have multiple entries here, which indicates that we detected a discrepancy in the plan.

Praveen2112 · 2026-03-09T14:08:03Z

core/trino-main/src/test/java/io/trino/sql/planner/assertions/MatchingDynamicFilters.java

+        return Optional.ofNullable(matching.get(alias));
+    }
+
+    public boolean containsUnresolvedAliases()


Is the method name a bit misleading, IIUC it looks for uniqueness of DynamicFilterId ? How does it correlate with unresolved alias, isn't more of a duplicate aliases ?

The goal is to have a 1:1 mapping between aliases and DynamicFilterIds. If that's not achieved, we have unresolved aliases.

Does that make sense or would you prefer a different name?

Praveen2112 · 2026-03-09T14:14:42Z

core/trino-main/src/test/java/io/trino/sql/planner/assertions/JoinMatcher.java

-
-        @CanIgnoreReturnValue
-        public Builder dynamicFilter(List<PlanMatchPattern.DynamicFilterPattern> expectedDynamicFilter)
+        public Builder addDynamicFilter(String alias, String buildAlias)


For consistency can we keep it as dynamicFilter ?

For consistency with other builders in the plan assertion framework, I chose addDynamicFilter rather than dynamicFilter or dynamicFilters.

See, for example, io.trino.sql.planner.assertions.WindowMatcher.Builder#addFunction(...).

It seems that the convention is: when setting a value, we drop the verb; when adding a value, we prefix it with add.

Previously, dynamic filter assertions either checked only the count of filters (numberOfDynamicFilters) or matched filters by expression without verifying the connection between producers and consumers. This made it possible for tests to pass even when dynamic filters were incorrectly wired. The new API uses test-defined aliases to explicitly link producers (at join nodes) with consumers (at filter nodes): join(INNER, builder -> builder .addDynamicFilter("DF", "BUILD_COL") .left(filter(TRUE, df -> df.addConsumer(c -> c.alias("DF").expression(BIGINT, "PROBE_COL")), tableScan(...))) .right(...)) During plan matching, the framework verifies that each alias maps to exactly one dynamic filter ID, ensuring producers and consumers are correctly paired.

Introduce MatchContext

55e71ca

Bundling parameters into a context object allows adding new fields without modifying all Matcher implementations.

cla-bot bot added the cla-signed label Feb 17, 2026

piotrrzysko force-pushed the df_testing branch from 2c3adda to 5c978fa Compare February 17, 2026 13:43

piotrrzysko marked this pull request as ready for review February 17, 2026 16:00

piotrrzysko requested review from findepi, losipiuk and raunaqmorarka February 17, 2026 16:06

raunaqmorarka requested review from kasiafi and martint February 19, 2026 05:39

findepi reviewed Feb 19, 2026

View reviewed changes

raunaqmorarka requested a review from Copilot February 19, 2026 11:23

Copilot started reviewing on behalf of raunaqmorarka February 19, 2026 11:24 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

piotrrzysko force-pushed the df_testing branch from 5c978fa to 5ab3b4b Compare February 26, 2026 10:52

raunaqmorarka reviewed Mar 9, 2026

View reviewed changes

core/trino-main/src/test/java/io/trino/sql/planner/assertions/DynamicFilterConsumerMatcher.java Show resolved Hide resolved

Praveen2112 reviewed Mar 10, 2026

View reviewed changes

piotrrzysko force-pushed the df_testing branch from 5ab3b4b to 2d96fa5 Compare March 13, 2026 09:30

piotrrzysko added 3 commits March 13, 2026 10:37

Fix formatting in PlanMatchPattern

744e9bc

Add alias-based dynamic filter assertions for semi-joins

9fb54c4

piotrrzysko force-pushed the df_testing branch from 2d96fa5 to 9fb54c4 Compare March 13, 2026 09:37

raunaqmorarka approved these changes Mar 13, 2026

View reviewed changes


		import static java.util.Objects.requireNonNull;

		public record DynamicFilterAlias(String alias)

Conversation

piotrrzysko commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary

Why?

Approach

API Change

Before

After

Additional context and related issues

Release notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

findepi commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

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!

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

piotrrzysko commented Feb 17, 2026 •

edited

Loading