[fix](runtime-filter) Preserve query semantics during partition pruning#65857
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review result: changes requested — one P1 correctness issue is attached inline.
Goal and data correctness: the FE NoneMovableFunction gate correctly prevents the new FE from sending partition-pruning metadata for assert_true, and the BE fallback correctly handles that function's INVALID_ARGUMENT for older-FE plans. The overall semantic goal is not complete, however: eligible deterministic targets can reject speculative LIST boundaries with other data-domain codes such as OUT_OF_BOUND, which still propagates and can turn a successful filtered query into an error.
Scope and design: the patch is small and focused. The grouped and single runtime-filter translator paths both use the classifier, late non-identity casts remain conservatively excluded, and ordinary runtime-filter generation is preserved.
Concurrency and lifecycle: the projection cache is fragment/operator-local, mutex-protected, and fed immutable boundaries/descriptor metadata. First-writer empty publication is conservative; cloned contexts, MIN/MAX wrappers, initial/late runtime-filter arrival, and conjunct/cache lock ordering did not reveal a correctness or deadlock issue. No new static initialization or ownership hazard was found.
Compatibility and parallel paths: new-FE/old-BE behavior is conservative for the marker-gated target, and new-BE/old-FE behavior is addressed for INVALID_ARGUMENT. The inline issue covers the remaining mixed-version/data-domain gap. CTE, set-operation, project-alias, grouped-filter, and single-filter paths were checked.
Tests and validation: the FE unit test, synthetic BE unit test, and regression test cover the marker gate and intended INVALID_ARGUMENT case, but do not cover another expression-domain status; a from_days(INT_MAX) regression would expose the remaining bug. Per the runner's review contract, no local build or test was run, and thirdparty/installed/bin/protoc is absent. Live Clang Formatter and CheckStyle pass; FE/BE/compile jobs were still pending at submission time. The macOS BE UT job failed before compilation because the runner exposed JDK 25 instead of required JDK 17.
Other checkpoints: no configuration, persistence/EditLog, data-write transaction, storage-format, FE-BE protocol-field, or observability change is introduced. The added type-bit lookup and one-time projection status check do not create a material performance concern.
User focus: no additional focus was provided; the full PR was reviewed.
Review completion: the main scan plus FE/planner, BE/tests, and risk-focused reviewers converged in round 2 with no unresolved candidates beyond the single inline issue.
TPC-H: Total hot run time: 29161 ms |
TPC-DS: Total hot run time: 176884 ms |
ClickBench: Total hot run time: 24.92 s |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: With runtime filter partition pruning enabled, a join target expression was eligible for partition pruning even when it contained a non-movable function such as assert_true. The pruning path then evaluated assert_true against LIST partition boundary values before normal row predicates ran, so a boundary belonging only to filtered-out rows could introduce an INVALID_ARGUMENT error. Without partition pruning, the same query returned 1. Reject target expressions containing NoneMovableFunction in the FE classifier so both modes preserve normal execution semantics and return 1.
### Release note
Fix runtime filter partition pruning to preserve query behavior for non-movable target expressions.
### Check List (For Author)
- Test: Regression test / Unit Test / Manual test
- RuntimeFilterPartitionPruneClassifierTest
- query_p0/runtime_filter/rf_partition_pruning
- Manual ON/OFF verification on port 9333
- Behavior changed: Yes. Target expressions containing non-movable functions no longer participate in runtime filter partition pruning.
- Does this need documentation: No
87ef50b to
4598414
Compare
|
run buildall |
TPC-H: Total hot run time: 29683 ms |
TPC-DS: Total hot run time: 177699 ms |
|
add impl pr into related pr list |
ClickBench: Total hot run time: 25.11 s |
|
/review |
github-actions
Bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
There was a problem hiding this comment.
Automated review result: changes requested — one P1 side-effect issue is attached inline.
Goal and data correctness: the new FE gate correctly prevents assert_true from being evaluated speculatively on LIST partition boundaries, and the unit plus regression coverage demonstrates that intended fix. The safety contract is still incomplete, however: sleep(partition_key) is deterministic by the current traits and is not NoneMovableFunction, so the pruning path can execute an arbitrary sleep for a selected partition whose rows are removed before normal target evaluation. An optional pruning optimization must not introduce this observable side effect.
Scope and design: the production change is small and focused, and containsType correctly recognizes implemented interfaces. The problem is the negative eligibility rule: NoneMovableFunction currently identifies only assert_true, while speculative boundary execution requires a positive guarantee that the target is total and side-effect-free (or an explicit safe allow-list).
Parallel paths and compatibility: grouped and single runtime-filter translation both call the classifier with aligned Nereids/legacy expressions. Project/alias, set-operation, and CTE rewrites preserve the function tree, and late non-identity casts remain conservatively excluded. No new Thrift field or storage format is added; omitting the existing optional pruning metadata is wire-compatible across FE/BE versions. The existing outdated from_days thread concerns an escaping domain-error status and was not duplicated; the new inline issue returns OK and changes execution through a side effect.
Concurrency and lifecycle: this FE planning-time trait check adds no shared mutable state, locking, thread, ownership, static-initialization, or lifecycle hazard. No configuration, persistence/EditLog, transaction, data-write, visible-version, memory-accounting, or observability change is introduced. The cached trait-bit lookup has negligible cost; the accepted sleep path is itself an unbounded performance/correctness problem.
Tests and validation: the added classifier unit test isolates the new gate, the ON/OFF regression queries both deterministically return 1, the profile assertion proves partition-pruning metadata is suppressed, session state is restored, and the .out additions are correctly ordered. A filtered-out nonzero LIST-boundary sleep regression is still needed. Per the runner contract, no local build or test was run. Live CI shows COMPILE, FE UT, CheckStyle, and NonConcurrent Regression passing. check_coverage_fe is failed; its TeamCity details return HTTP 401 from this runner, so the cause could not be verified here.
User focus: no additional focus was provided; the full four-file PR was reviewed.
Review completion: the main risk scan, two normal full-review agents, and a separate risk-focused agent converged in Round 2 on this exact candidate, line anchor, body, live head, and existing-thread set. Every other suspicious point was dismissed with concrete code evidence, and no unresolved candidate remains beyond the inline issue.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
With runtime filter partition pruning enabled, a join target expression was eligible for partition pruning even when it contained a non-movable function such as
assert_true. The pruning path then evaluatedassert_trueagainst LIST partition boundary values before normal row predicates ran. A boundary belonging only to filtered-out rows could therefore introduce anINVALID_ARGUMENTerror, while the same query returned1with partition pruning disabled.The FE runtime-filter partition-prune classifier now rejects target expressions containing
NoneMovableFunction. This prevents speculative partition-boundary evaluation ofassert_true; both partition-pruning modes preserve normal query semantics and return1.Release note
Fix runtime filter partition pruning to preserve query behavior for non-movable target expressions.
Check List (For Author)
Tested with:
RuntimeFilterPartitionPruneClassifierTest: 6 tests passedquery_p0/runtime_filter/rf_partition_pruning: 1 suite passedManual comparison on FE port 9333: both
enable_runtime_filter_partition_prune=falseandtruereturn1Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)