[refactor](runtime filter) merge runtime filter generator v1 and v2#62383
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
3 similar comments
|
run buildall |
|
run buildall |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
|
/review |
|
run cloud_p0 |
|
run vault_p0 |
|
run external |
englefly
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Blocking issues found:
- The new set-operation runtime-filter pushdown now falls through a generic child-recursion path and no longer preserves the old V2 operator guards. That makes it possible to push a set-op RF below operators like
TopN/LIMIT(and other guarded nodes), which is not semantics-preserving. - The merged set-op path no longer checks
canPushDownRuntimeFilter()and now accepts unsupportedPhysicalRelationtargets. A single invalid target (for examplePhysicalOneRowRelation) can poison the whole legacy RF group during translation and drop otherwise valid sibling targets. - The PR includes local
regression-test/conf/regression-conf.groovyport changes, which are environment-only and should not be committed.
Critical checkpoints:
- Goal of current task: Merge RF V1/V2 and preserve runtime-filter behavior. Not fully achieved; the new set-op pushdown introduces regressions and the PR also includes unrelated environment config changes.
- Modification size/focus: Not fully focused because local regression-test configuration was committed.
- Concurrency: No new concurrency-sensitive code or lock-order issue was identified in the reviewed paths.
- Lifecycle/static initialization: Not applicable in the touched code.
- Configuration items: No new product config was added, but local test-environment config was modified and committed accidentally.
- Compatibility: No FE/BE protocol or storage-format incompatibility was identified in the reviewed paths.
- Parallel code paths: Not fully preserved; the old V2 set-op pushdown guards for blocked operators/unsupported targets were lost in the merged path.
- Special conditional checks: Missing guard(s) around unsupported relation targets and operator-specific pushdown boundaries.
- Test coverage: The updated shape/regression tests do not cover set-op children with
TopN/LIMITor constant/empty children, so these regressions are currently untested. - Observability: No additional observability changes appear necessary here.
- Transaction/persistence: Not applicable.
- Data writes/atomicity: Not applicable.
- FE-BE variable passing: No new variable-passing issue was identified in the reviewed paths.
- Performance: There is also a performance regression risk because one invalid set-op target can cause the whole RF group to be dropped, removing valid sibling filters.
- Other issues: None beyond the inline findings.
| Map<Slot, Expression> replaceMap = ExpressionUtils.generateReplaceMap(project.getProjects()); | ||
| Expression newTarget = targetExpr.rewriteDownShortCircuit( | ||
| e -> replaceMap.getOrDefault(e, e)); | ||
| if (newTarget.getInputSlots().size() == 1) { |
There was a problem hiding this comment.
This fallback recursion changes semantics for set-operation RF pushdown. In the deleted V2 visitor, PhysicalTopN stopped pushdown entirely and PhysicalWindow/join nodes had extra guards; here we now recurse through any unmatched node whose child still exposes the slot. That means a query like select a from t1 except (select a from t2 order by b limit 10) can push the RF below the TopN, filtering rows before the top-10 is chosen and therefore changing which rows participate in the EXCEPT. Please keep the operator-specific stop conditions instead of using the generic recurse-into-children fallback here.
| /** | ||
| * Push down runtime filter for set operations (INTERSECT/EXCEPT) by tree traversal. | ||
| * Unlike join RF push-down which uses aliasTransferMap, this method rewrites the target | ||
| * expression through projects and resolves to scans directly. |
There was a problem hiding this comment.
This branch now accepts every PhysicalRelation except PhysicalSchemaScan, but the old implementations only targeted relations that actually support runtime-filter pushdown (PhysicalCatalogRelation / PhysicalCTEConsumer, or canPushDownRuntimeFilter() == true). With the new code, unsupported targets such as PhysicalOneRowRelation or PhysicalEmptyRelation can be added to the same RF group. Later, RuntimeFilterTranslator.createLegacyRuntimeFilter() aborts the whole group on the first target whose targetSlotRef is missing (hasInvalidTarget = true; break;), so one constant/empty child can silently drop valid sibling targets too. A query like select a from t1 intersect select 1 as a intersect select a from t2 now loses the RF on t2 because the constant child poisons the shared filter.
| // add allowLoadLocalInfile so that the jdbc can execute mysql load data from client. | ||
| jdbcUrl = "jdbc:mysql://127.0.0.1:9030/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round" | ||
| targetJdbcUrl = "jdbc:mysql://127.0.0.1:9030/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round" | ||
| jdbcUrl = "jdbc:mysql://127.0.0.1:29031/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round" |
There was a problem hiding this comment.
These are local runner/worktree ports from the test environment, not part of the feature. AGENTS.md explicitly says environment-only changes like conf/ updates must not be committed. Please drop this file from the PR.
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
|
/review |
There was a problem hiding this comment.
I re-checked the updated head. The local regression-conf.groovy change and the broad non-scan target admission issue are fixed, but 2 blocking issues remain.
RuntimeFilterGenerator.pushDownSetOpRuntimeFilter()still stops atPhysicalTopNbut not atPhysicalLimit, so set-operation RF can still be pushed below a bareLIMIT, changing which rows participate inINTERSECT/EXCEPT.- The set-operation relation gate still admits
PhysicalSchemaScanviaPhysicalCatalogRelation, even thoughPhysicalSchemaScan.canPushDownRuntimeFilter()is explicitlyfalseand the join RF path still rejects schema scans.
Critical Checkpoints
- Goal of current task: Partially accomplished. The V1/V2 merge is mostly in place, but set-op pushdown still has one missing semantic guard (
PhysicalLimit) and still targets an unsupported scan type (PhysicalSchemaScan). - Modification size/focus: Large refactor across generator, pushdown, translator, context, plan nodes, and many shape files. The scope matches the refactor, but regression risk remains high.
- Concurrency: No new concurrency, locking, or lifecycle hazard found in the reviewed FE paths.
- Special lifecycle/static initialization: No relevant lifecycle or static-init issue found.
- Configuration items: No product configuration issue remains on the current head.
- Compatibility/incompatible changes: No FE/BE protocol or storage-format incompatibility found in the reviewed runtime-filter descriptor path itself.
- Parallel code paths: Join and set-op RF generation still diverge in guard logic; the set-op path still lacks the
PhysicalLimitstop and still bypasses thecanPushDownRuntimeFilter()contract for schema scans. - Special conditional checks: Incomplete.
PhysicalLimitis still unguarded and the relation gate is still broader than the supported-target contract. - Test coverage: The updated shape outputs still do not cover a set-op child with bare
LIMITor aninformation_schematarget, so both regressions remain untested. - Observability: No additional observability change appears necessary.
- Transaction/persistence: Not applicable.
- Data writes/atomicity: Not applicable.
- FE-BE variable passing: No new variable-passing issue identified in the reviewed paths.
- Performance: No new standalone performance blocker identified beyond the behavioral issues above.
- Other issues: None beyond the inline findings.
| * expression through projects and resolves to scans directly. | ||
| */ | ||
| private boolean pushDownSetOpRuntimeFilter(Plan plan, Expression targetExpr, | ||
| Expression srcExpr, PhysicalSetOperation setOp, RuntimeFilterContext ctx, |
There was a problem hiding this comment.
This still admits PhysicalSchemaScan, because it extends PhysicalCatalogRelation. That bypasses the existing support contract: PhysicalSchemaScan.canPushDownRuntimeFilter() is explicitly false ("currently be doesn't support schema scan rf"), and the join RF path still rejects schema scans in RuntimeFilterPushDownVisitor.visitPhysicalRelation(). A set-operation query over information_schema can therefore build an RF for a scan type we already consider unsupported. Please gate this on relation.canPushDownRuntimeFilter() or explicitly exclude PhysicalSchemaScan here.
| Expression newTarget = targetExpr.rewriteDownShortCircuit( | ||
| e -> replaceMap.getOrDefault(e, e)); | ||
| if (newTarget.getInputSlots().size() == 1) { | ||
| return pushDownSetOpRuntimeFilter(project.child(), newTarget, |
There was a problem hiding this comment.
PhysicalLimit still falls through to the generic recursion below, because only PhysicalTopN is stopped here. LogicalLimitToPhysicalLimit can still leave a plain PhysicalLimit under the target side of a set operation, and then this method pushes the RF below the limit, filtering rows before the limit chooses its prefix. That changes which rows participate in INTERSECT/EXCEPT. Please add a PhysicalLimit guard before the generic fallback.
There was a problem hiding this comment.
limit 是可以下推的。topn不能下推是因为下推后会改变topn的输出结果。
|
/review |
|
run buildall |
There was a problem hiding this comment.
I’m requesting changes because this merge changes join runtime-filter behavior in two places that don’t look behavior-preserving.
RuntimeFilterPushDownVisitornow treats any one-slot probe expression as valid for join RF pushdown. That re-enables filters on arbitrary functions likesubstring(...), which v1 intentionally rejected for scan CPU reasons. The updatedshape_check/.../query8.outbaselines already show these new substring RFs.- The refactor removed the only producer for
RuntimeFilterContext.expandedRF, butRuntimeFilterPrunerstill depends on that metadata whenexpand_runtime_filter_by_inner_joinand RF pruning are enabled. As a result the expanded-target pruning path becomes unreachable and extra transitive targets stop being pruned.
Critical checkpoints:
- Goal: Partially achieved. The set-op/join generation paths are unified, but the merge is not behavior-preserving on the join side.
- Scope: Mostly focused, but it also broadens join RF semantics and disables an existing pruning path, so this is not a pure refactor.
- Concurrency: No new concurrency concerns found; this planner/post-process path is single-threaded.
- Lifecycle/config/compatibility: No new lifecycle, config, or compatibility issues found.
- Parallel paths: The old join-only expression guard and expanded-RF pruning path were not carried forward into the unified implementation.
- Conditional logic: The removed probe-expression restriction had an explicit performance rationale and now lacks any replacement guard.
- Tests: Regression outputs were updated, but no FE test was added for the new join-expression RF behavior or for expanded-RF pruning with pruning enabled. I could not run
./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.RuntimeFilterTesthere becausethirdparty/installed/bin/protocis missing in the runner. - Observability: Unchanged.
| */ | ||
| public boolean isValid() { | ||
| return finalTarget != null && probeSlot != null; | ||
| return probeExpr.getInputSlots().size() == 1; |
There was a problem hiding this comment.
This broadens join/NLJ runtime-filter targets from the old slot-or-cast-on-slot rule to any one-slot expression. That is not just a refactor: it re-enables expensive probe expressions like substring(...)/abs(...) on scan nodes. The updated shape_check/.../query8.out baselines already show a new substring(s_zip, 1, 2) RF that v1 deliberately refused to build. If the goal here is only to reuse the v2 pushdown walker, we still need the old join-side guard (or some equivalent cost gate) so we do not regress scan CPU by evaluating arbitrary functions on every row.
| } | ||
| if (newTarget != null && newTarget.getInputSlots().size() == 1 | ||
| && !newTarget.equals(ctx.srcExpr)) { | ||
| PushDownContext expanded = ctx.withNewProbeExpression(newTarget); |
There was a problem hiding this comment.
The old implementation recorded RuntimeFilterContext.ExpandRF metadata here when expand_runtime_filter_by_inner_join created a transitive target. RuntimeFilterPruner.visitPhysicalHashJoin() still relies on getExpandRfByJoin(join) in the default enableRuntimeFilterPrune=true path, but after this refactor nothing ever appends to expandedRF anymore. That makes the expanded-target pruning branch unreachable and changes the default behavior for join chains whose build side is an EffectiveSrcType.REF. If the pruning logic is still needed, this metadata has to be preserved or the pruner needs to be updated in the same PR.
FE Regression Coverage ReportIncrement line coverage |
englefly
force-pushed
the
merge-rf-gen
branch
2 times, most recently
from
8b96ccd to
e3f85bc
Compare
|
/review |
|
run buildall |
There was a problem hiding this comment.
I re-checked the updated head and found one blocking issue remaining.
RuntimeFilterGenerator.pushDownSetOpRuntimeFilter()still misses the newPhysicalRepeat/ grouping-sets safety rule.RuntimeFilterPushDownVisitor.visitPhysicalRepeat()now blocks join RF pushdown unless the probe slot is present in all grouping sets, but the newINTERSECT/EXCEPTtraversal still falls through to generic child recursion and can push the RF belowRepeat, which can change results for set-op children containingGROUPING SETS.
Critical Checkpoints
- Goal of current task: Partially accomplished. The V1/V2 merge is mostly in place, but the new grouping-sets safeguard is only applied on the join path, not on the newly added set-op path.
- Modification size/focus: Mostly focused on runtime-filter unification and cleanup.
- Concurrency: No new concurrency, locking, or lifecycle hazard found in the reviewed FE paths.
- Lifecycle/static initialization: No relevant lifecycle or static-init issue found.
- Configuration items: No new product configuration issue identified.
- Compatibility/incompatible changes: No FE/BE protocol or storage-format incompatibility found in the reviewed paths.
- Parallel code paths: Not fully preserved;
RuntimeFilterPushDownVisitorgained aPhysicalRepeatguard, butRuntimeFilterGenerator.pushDownSetOpRuntimeFilter()did not. - Special conditional checks: Missing
PhysicalRepeat/ grouping-sets guard on the set-op pushdown path. - Test coverage: The new FE tests cover the join-side
Repeatcase, but there is still noINTERSECT/EXCEPTcoverage for the same grouping-sets scenario. - Observability: No additional observability change appears necessary.
- Transaction/persistence: Not applicable.
- Data writes/atomicity: Not applicable.
- FE-BE variable passing: No new variable-passing issue identified.
- Performance: No additional standalone performance blocker identified beyond the behavioral issue above.
- Other issues: None beyond the inline finding.
| Map<Slot, Expression> replaceMap = ExpressionUtils.generateReplaceMap(project.getProjects()); | ||
| Expression newTarget = targetExpr.rewriteDownShortCircuit( | ||
| e -> replaceMap.getOrDefault(e, e)); | ||
| if (newTarget.getInputSlots().size() == 1) { |
There was a problem hiding this comment.
Can we add the same PhysicalRepeat guard here that RuntimeFilterPushDownVisitor.visitPhysicalRepeat() now has? The new set-op traversal still falls through to this generic recursion for PhysicalRepeat, so an INTERSECT/EXCEPT child containing GROUPING SETS can push an RF below Repeat even when the filtered slot is not present in every grouping set. That is the same wrong-result case the new join-side tests (testRuntimeFilterBlockByGroupingSetsPartialColumn / ...CommonColumn) are fixing, but there is no corresponding guard or coverage on the set-op path yet.
|
/review |
|
run buildall |
There was a problem hiding this comment.
I found blocking issues in the new grouped translation path.
RuntimeFilterTranslator#createLegacyRuntimeFilters()now groups filters only by(srcExpr, type). That is not sufficient for nested-loop runtime filters. For example,A.x < B.y AND A.z > B.yproduces twoMIN_MAXfilters with the same source exprB.ybut differentTMinMaxRuntimeFilterType(MAXvsMIN). The grouped path keeps onlyhead.gettMinMaxType()/head.getExprOrder()/head.isBitmapFilterNotIn(), so one target is translated with the wrong semantics.createLegacyRuntimeFilterFromGroup()returns as soon as any member of the group is listed inignore_runtime_filter_ids. That drops the entire grouped legacy filter, including non-ignored members. The old V2 translator filtered ignored IDs before grouping, so this is a regression.
Critical checkpoints:
- Goal of the change: Partially met. The PR unifies join/set-op RF generation, but the grouped translator is not correct end to end for some NLJ / ignore-id cases.
- Scope and focus: Mostly focused on runtime-filter unification, but the new translator helper changes behavior across hash join, nested loop join, and set-op translation.
- Concurrency: No new concurrency or locking concerns found in this FE path.
- Lifecycle / static init: No special lifecycle or static-init issues found.
- Config items: No new configs.
- Compatibility: No storage/protocol format changes, but runtime-filter FE->BE semantics change because separate filters are now merged differently.
- Parallel code paths: Join, NLJ, set-op, CTE pushdown, and scan translation were updated; the new grouped translation path is where the regressions are.
- Special conditional checks: The new
Repeatand non-numeric pushdown guards are documented and reasonable. - Test coverage: Added FE unit/regression coverage for set-op pushdown, repeat, and expression pushdown, but there is no coverage for
ignore_runtime_filter_idswith grouped filters or for NLJ filters that share the same source expr but differ in min/max or bitmap-not semantics. - Modified test results: Many
.outfiles changed; I did not re-run the FE/regression suites in this review environment. - Observability: No additional logging/metrics appear required.
- Transaction / persistence / data writes: Not applicable for this FE-only refactor.
- FE/BE variable passing: Reuses existing thrift fields, but the new grouping now reuses one descriptor across filters that may not share the same
exprOrder/min_max_typesemantics. - Performance: The grouping reduces legacy filter count, but correctness should come first here.
- Other issues: None beyond the blocking translation regressions above.
Requesting changes until the grouped translation preserves per-filter semantics.
| List<RuntimeFilter> others = new ArrayList<>(); | ||
| group.add(head); | ||
| for (int i = 1; i < remaining.size(); i++) { | ||
| RuntimeFilter rf = remaining.get(i); |
There was a problem hiding this comment.
Grouping only on (srcExpr, type) is too coarse here. After this PR, createLegacyRuntimeFilters() is also used for nested-loop RFs, and those can share the same source/type while still carrying different semantics. A concrete case is A.x < B.y AND A.z > B.y: both filters have srcExpr = B.y and type = MIN_MAX, but one must be MAX and the other MIN. This helper merges them and later keeps only head.gettMinMaxType() / head.getExprOrder() / head.isBitmapFilterNotIn(), so one target is translated with the wrong runtime-filter semantics.
| if (ConnectContext.get() != null) { | ||
| for (RuntimeFilter rf : group) { | ||
| if (ConnectContext.get().getSessionVariable() | ||
| .getIgnoredRuntimeFilterIds().contains(rf.getId().asInt())) { |
There was a problem hiding this comment.
This return drops the entire grouped legacy filter if any member is listed in ignore_runtime_filter_ids. That regresses the old behavior: the previous V2 translator filtered ignored RF IDs before grouping, so non-ignored siblings with the same (srcExpr, type) were still translated. With this code, ignoring one RF can accidentally disable every other grouped target from the same builder.
|
/review |
There was a problem hiding this comment.
Reviewed the merged runtime-filter path across generation, pushdown, pruning, translation, and set-operation handling. I did not find a blocking correctness regression in the join, set-op, or CTE flow.
Critical checkpoint conclusions:
- Goal / correctness: The PR achieves the stated goal of merging the V1 and V2 runtime-filter generation paths and extending the unified pushdown flow to set operations. The added FE and regression coverage exercises set-op RF generation, grouped legacy translation, min/max grouping, ignored RF ids, repeat and grouping-set blocking, and probe-expression restrictions.
- Scope / focus: The refactor is broad but stays focused on the runtime-filter pipeline and removes the obsolete V2-only path instead of leaving two parallel implementations.
- Concurrency / lifecycle: This code runs in FE plan post-processing and translation. I did not find new concurrent-state, locking, or lifecycle hazards in the modified flow.
- Config / compatibility: No new persisted state or FE-BE protocol fields are introduced. The change reuses the existing planner and thrift runtime-filter structures, including the existing set-operation execution path.
- Parallel paths / conditions: Join, set-op, CTE pushdown, pruning, and legacy translation paths were updated consistently. The added guards around repeat, window, topN, and non-numeric scan expressions look justified.
- Tests: The new and updated FE tests and regression outputs are aligned with the intended behavior. I could not rerun FE unit tests locally in this runner because Maven could not resolve org.apache.doris:fe-foundation:1.2-SNAPSHOT.
Residual risk: the target-null translation branch in the grouped legacy translator is not directly covered by a dedicated new test, although I did not find a concrete correctness failure from the current call ordering.
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Merge the Nereids runtime filter generator flows into a single implementation by folding the old V2 path into the V1 planner flow, unifying hash join and set operation pushdown with one visitor, fixing repeat/grouping sets and translator grouping edge cases, simplifying nereids runtime filters to single-target storage while keeping binary compatibility for incremental builds, updating shape-check baselines for the new runtime filter print format, and cleaning up obsolete helper infrastructure.
### Release note
Improve Nereids runtime filter generation and translation correctness, including set operation pushdown, repeat/grouping sets handling, legacy runtime filter translation semantics, and runtime filter explain output formatting.
### Check List (For Author)
- Test: FE unit test
- mvn test -pl fe-core -Dtest="org.apache.doris.nereids.postprocess.RuntimeFilterTest" -T4
- Behavior changed: Yes (runtime filter generation, pushdown, translation, and explain output are unified and several correctness bugs are fixed)
- Does this need documentation: No
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
There was a problem hiding this comment.
Re-reviewed current head 45e9626. I did not find a blocking correctness regression in the merged runtime-filter path.
Critical checkpoint conclusions:
- Goal / correctness: The current head achieves the V1/V2 merge and the unified join, set-operation, CTE pushdown, pruning, and grouped legacy-translation flow looks consistent end to end. I did not find a concrete wrong-result path in the reviewed join, set-op, NLJ, or CTE cases.
- Scope / focus: The refactor stays focused on runtime-filter unification and removal of the obsolete V2-only path.
- Concurrency: No new concurrency, locking, or lifecycle hazard found; this FE post-process / translation path is single-threaded.
- Lifecycle / static init: No relevant special lifecycle or static-initialization issue found.
- Config / compatibility: No new persisted state or FE-BE protocol fields are introduced; the change reuses the existing planner and thrift runtime-filter structures.
- Parallel paths / conditions: Join, set-op, translator, and pruning paths are aligned on the current head, including the repeat, window, topN, and non-numeric scan guards.
- Test coverage: The added FE and regression coverage exercises set-op pushdown, grouped legacy translation, ignore-id handling, min/max grouping, repeat / grouping-sets blocking, and scan-expression restrictions. I did not rerun FE or regression suites in this environment.
- Observability / persistence / writes / FE-BE variable passing: No additional issue found in the reviewed paths.
- Performance: No new blocker identified in the reviewed code.
Residual risk: there is still limited prune-enabled coverage for join and set-operation interactions, but I did not confirm a correctness failure on the current head.
|
run buildall |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
…pache#62383) ### What problem does this PR solve? v1 is an early version that generates runtime filters for join based on aliasTransferMap. However, aliasTransferMap is not conducive to the functional expansion of runtime filters (RF), so a push-down process was introduced instead. Version 2 adopts the push-down process to overcome the limitations imposed by aliasTransferMap.In the master branch, v2 is only used to generate runtime filters for set operators. The purpose of this PR is to unify the generation of runtime filters for both join and set operators using the push-down process.
### What problem does this PR solve? Issue Number: None Related PR: apache#62383 Problem Summary: Runtime filters pushed from multiple CTE consumers into a shared CTE producer only checked for the same build-side source expression. After runtime filter generation was unified in apache#62383, OR expansion can create multiple consumer-side probe expressions such as pk + 6 and pk - 1 for the same shared producer. Mapping those filters to only the producer slot loses the expression difference and can apply one branch-specific filter to the shared scan, producing wrong results for anti joins. This patch requires all producer-side target expressions to be identical before pushing filters into the shared producer, and preserves the full producer target expression when doing the pushdown. ### Release note Fix incorrect query results caused by unsafe runtime filter pushdown into shared CTE producers. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.RuntimeFilterTest#testPushSharedCteRuntimeFilterOnlyForSameProducerTargetExpression - Behavior changed: Yes. The optimizer no longer pushes branch-specific runtime filters into a shared CTE producer when consumer probe expressions differ, which restores correct query results. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#62383 Problem Summary: Runtime filters pushed from multiple CTE consumers into a shared CTE producer only checked for the same build-side source expression. After runtime filter generation was unified in apache#62383, OR expansion can create multiple consumer-side probe expressions such as pk + 6 and pk - 1 for the same shared producer. Mapping those filters to only the producer slot loses the expression difference and can apply one branch-specific filter to the shared scan, producing wrong results for anti joins. This patch requires all producer-side target expressions to be identical before pushing filters into the shared producer, and preserves the full producer target expression when doing the pushdown. ### Release note Fix incorrect query results caused by unsafe runtime filter pushdown into shared CTE producers. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.RuntimeFilterTest#testPushSharedCteRuntimeFilterOnlyForSameProducerTargetExpression - Behavior changed: Yes. The optimizer no longer pushes branch-specific runtime filters into a shared CTE producer when consumer probe expressions differ, which restores correct query results. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#62383 Problem Summary: Runtime filters pushed from multiple CTE consumers into a shared CTE producer only checked for the same build-side source expression. After runtime filter generation was unified in apache#62383, OR expansion can create multiple consumer-side probe expressions such as pk + 6 and pk - 1 for the same shared producer. Mapping those filters to only the producer slot loses the expression difference and can apply one branch-specific filter to the shared scan, producing wrong results for anti joins. This patch requires all producer-side target expressions to be identical before pushing filters into the shared producer, and preserves the full producer target expression when doing the pushdown. It also adds a regression case with two shared CTE consumers whose producer-side target expressions are cast(pk as bigint) + 6 and cast(pk as bigint) - 1. ### Release note Fix incorrect query results caused by unsafe runtime filter pushdown into shared CTE producers. ### Check List (For Author) - Test: Unit Test / Regression test - ./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.RuntimeFilterTest#testPushSharedCteRuntimeFilterOnlyForSameProducerTargetExpression - Added regression-test/suites/query_p0/runtime_filter/cte-runtime-filter.groovy coverage for shared CTE runtime filters with different producer-side target expressions. Not run locally because no FE was listening on the configured regression port 9030. - Behavior changed: Yes. The optimizer no longer pushes branch-specific runtime filters into a shared CTE producer when consumer probe expressions differ, which restores correct query results. - Does this need documentation: No
4 participants
What problem does this PR solve?
v1 is an early version that generates runtime filters for join based on aliasTransferMap. However, aliasTransferMap is not conducive to the functional expansion of runtime filters (RF), so a push-down process was introduced instead.
Version 2 adopts the push-down process to overcome the limitations imposed by aliasTransferMap.In the master branch, v2 is only used to generate runtime filters for set operators.
The purpose of this PR is to unify the generation of runtime filters for both join and set operators using the push-down process.
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)