[feature](runtime filter) Support single-column runtime filter bucket pruning - #65837
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 25th, 2026 6:03 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
morrySnow
left a comment
There was a problem hiding this comment.
Thanks for this PR! The implementation is well-structured and integrates cleanly with the existing runtime-filter infrastructure. Below is a summary of my overall findings; detailed inline comments follow.
Overview: This PR adds the ability for exact IN runtime filters on single-column HASH-distributed OLAP tables to skip scanning tablets whose bucket index doesn't match any filter value. The FE classifier determines eligibility, and the BE pruner materializes filter values into hashes and computes which buckets to skip. The feature is gated behind enable_runtime_filter_bucket_prune (default true).
Strengths:
- Clean separation between FE eligibility classification and BE pruning logic
- Good test coverage (FE unit tests, BE unit tests, regression test)
- Proper concurrency handling with shared_mutex
- Conservatively falls back when prerequisites aren't met (Bloom, composite hash, etc.)
- Efficient: caches selected buckets by bucket_num to avoid recomputation
Key issues to address:
- Asymmetric column comparison in
sameColumn()— uses different name methods for target vs distribution columns, which could miss pruning opportunities - NULL handling in
materialize_hashes— inserting a default value for NULL adds a spurious hash that prevents optimal pruning - A few minor style/convention items noted inline
morrySnow
left a comment
There was a problem hiding this comment.
Inline comments on specific code locations (see the overall review for summary).
0ff4ba8 to
1a8eff7
Compare
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime filter bucket pruning compared distribution columns with an asymmetric name fallback and could accept columns with conflicting unique IDs. Compare set unique IDs first and use symmetric base-column names only when IDs are unavailable. Clarify fixed-width and null-aware hashing contracts, and cover both behaviors with focused tests. ### Release note None ### Check List (For Author) - Test: Unit Test - FE RuntimeFilterBucketPruneClassifierTest (8 tests) - BE RuntimeFilterBucketPrunerTest (6 tests) - Behavior changed: Yes (bucket pruning is disabled when target and distribution unique IDs conflict; query results are unchanged) - Does this need documentation: No
|
/review |
There was a problem hiding this comment.
Review status: complete after two convergence rounds. Requesting changes for the correctness issue described inline.
Critical checkpoints:
- Goal and proof: The change aims to use exact single-column runtime filters to skip HASH-distribution buckets. The supplied FE, BE, and regression coverage exercises the ordinary base-table path, NULL behavior, composite-distribution rejection, varying bucket counts, and the disable gate, but it does not cover selected rollups whose index-local column IDs collide. That reachable path can prune the bucket containing a matching row, so the implementation does not yet meet the data-correctness goal.
- Scope and clarity: The change is otherwise focused on bucket pruning and its FE/BE metadata plumbing.
- Concurrency and lifecycle: I traced initial and late runtime-filter publication, local/global merging, scanner scheduling/closure, and the relevant locks. Publication is serialized, pruning is monotone, and I found no additional concurrency, lifecycle, or lock-order issue.
- Configuration and compatibility: The forwarded session variable and optional Thrift fields degrade to no pruning when metadata is absent, including rolling-upgrade directions. No additional compatibility issue was found.
- Parallel paths and conditions: Grouped targets, selected partitions, per-partition bucket counts, scheduler/colocate/bucket-shuffle range propagation, exact-set versus Bloom behavior, and disabled/partial metadata paths were checked. No additional issue was substantiated.
- Tests and results: The checked-in expectations are consistent with the covered paths. Please add a same-ID/different-base-column selected-rollup test for the blocking case. I did not run builds or tests because this review runner's instructions prohibit them.
- Performance and observability: Exact-set materialization is bounded by the configured IN-filter limit, and the added pruning counter provides useful visibility. No material additional issue was found.
- Transactions, persistence, and writes: Not applicable; this is execution-time scan pruning and does not change stored data, transaction state, or edit-log formats.
- User focus: No additional focus was provided, so the full PR was reviewed.
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime filter bucket pruning treated column unique IDs as table-wide identities, but Doris assigns them independently per materialized index. A selected rollup column could therefore share an ID with a different base distribution column and incorrectly enable bucket pruning, causing matching rows to be skipped. Match columns by logical base-column name across index namespaces and require exact type equality. Add focused rollup ID-collision and type-mismatch coverage. ### Release note None ### Check List (For Author) - Test: Unit Test - FE RuntimeFilterBucketPruneClassifierTest (10 tests) - Behavior changed: Yes (unsafe bucket pruning is disabled for cross-index column ID collisions and type mismatches) - Does this need documentation: No
|
/review |
There was a problem hiding this comment.
Review status: complete after three convergence rounds. Requesting changes for the three P2 performance issues described inline.
The functional goal is sound: exact IN runtime filters on a direct single-column HASH target can conservatively prune whole tablets, while composite distributions, non-exact representations, over-limit sets, and disabled sessions fall back. FE classifier tests, BE unit tests, and the regression cover the basic eligible/fallback, INT/NULL, heterogeneous-bucket, result, counter, composite, and session-off contracts. The change is otherwise focused, but it introduces avoidable work in the FE point-query path and both BE late-update and common scanner paths.
Concurrency and lifecycle: initial pruning precedes scanner creation; late filters are serialized by _conjuncts_lock; the monotonic pruned-tablet set is protected by _prune_mutex; and scanner checks cannot turn an in-flight block into row loss. No inverse lock order, deadlock, partial publication, close/use-after-free, circular ownership, or static-initialization issue was found. The remaining lock concerns are performance defects: repeated old-filter work stays under _conjuncts_lock, and permanently ineligible scans repeatedly take _prune_mutex in shared mode.
Configuration, compatibility, and propagation: the session variable is forwarded per query and needs no mid-query refresh. Current FE writes paired range metadata and eligible target IDs under the same gate; old-FE/new-BE and new-FE/old-BE executions conservatively fall back through appended optional Thrift fields. Ordinary and distributed Nereids coordinator paths preserve the annotated scan-range objects. No function-symbol or storage-format incompatibility was found.
Parallel and special paths: selected rollups, per-partition bucket counts, sampled/specified/distribution-pruned ranges, broadcast/global/local-exchange/local-aligned filters, parallel scanner fan-out, query cache, and exact-to-Bloom fallback were traced and found correctness-safe. Conditions fail conservatively, new Status results are checked, temporary hash state is memory-accounted, and BucketsPrunedByRuntimeFilter provides activation observability. Transaction, EditLog, failover, visible-version, delete-bitmap, and data-write concerns are not applicable to this query-local read optimization.
Tests and results: the submitted tests establish the feature-level contract and the checked-in output is consistent and deterministic. A production-routing type matrix and late/local/multi-driver integration cases would strengthen coverage, but code inspection found no concrete defect at those boundaries, so they are not separate blocking findings. No builds or tests were run in this review-only environment.
Performance findings:
- Prepared short-circuit point lookups rebuild and retain two all-tablet bucket maps despite having no runtime-filter producer.
- Staggered late filters reprocess all previously appended exact filters under the conjunct lock, producing triangular work.
- Every scheduled OLAP scan block takes two bucket-pruner shared locks even when the scan can never use bucket pruning.
User focus: no additional focus was supplied; the full PR scope and all applicable critical checkpoints were reviewed.
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime-filter partition and bucket pruning exposed separate scanner APIs, reprocessed all accumulated filters while holding the conjunct lock, retained duplicate all-tablet metadata for ordinary scans, and built that metadata for point queries. With F staggered filters, late-arrival pruning performed F(F+1)/2 filter passes. Unify the scheduler-facing API as is_pruned_by_runtime_filter(), process only the newly appended immutable filter slice outside the conjunct lock so the work is F passes, compact bucket number and sequence into one map, exclude point queries from that map, and bypass the bucket-pruner lock for ineligible scans. ### Release note None ### Check List (For Author) - Test: Unit Test - BE RuntimeFilterBucketPrunerTest and RuntimeFilterPartitionPrunerTest (17 tests) - FE RuntimeFilterBucketPruneClassifierTest (10 tests) - BE and FE Release compilation - BE clang-format and format check - Behavior changed: No - Does this need documentation: No
|
/review |
Performance validationTested commit A/B setup
The result was identical in both modes ( TPCH SF100 sanity checkI also ran the runtime-filter join against the existing SF100 This validates that the optimization preserves correctness and materially reduces tablet/scanner setup overhead when runtime-filter selectivity identifies a small subset of hash buckets. |
|
run buildall |
There was a problem hiding this comment.
Review completed after two full-scope convergence rounds. One blocking concurrency issue is reported inline.
Checkpoint conclusions:
- Goal and proof: the PR implements conservative exact-IN bucket pruning for eligible single-column HASH scans. Focused FE/BE unit tests and enabled, disabled, and composite-distribution regression coverage exercise the intended behavior.
- Scope and clarity: the changes are focused on eligibility, optional metadata, local-state pruning, scheduler checks, counters, and tests.
- Concurrency and lifecycle: scanner-worker entry points, helper/conjunct locks, projected-boundary caching, pruned-set publication, scheduling, and teardown were traced. The per-pruner locks and publication paths are otherwise sound, but releasing
_conjuncts_lockexposes the accepted clone/open-versus-projection/execute race. No lock-order cycle was found. - Configuration and compatibility: the new switch is query-scoped and forwarded consistently. Appended optional Thrift fields make old-FE/new-BE and new-FE/old-BE combinations conservatively skip bucket pruning when metadata is absent.
- Parallel and special paths: initial and late filters, split scanners, point queries, cloud/shared-nothing execution, sampling/specified tablets, grouped targets, and varying per-partition bucket counts were checked; they either preserve provenance or fall back conservatively.
- Correctness and performance: admitted HybridSet values use the same type-specific CRC family as tablet routing, while Bloom/oversized/casted/composite cases are fenced. Newly arrived filters are processed once and ineligible scans bypass bucket locking.
- Tests and observability: changed tests/results are deterministic and the new counters expose partition/tablet/bucket effects. A deterministic clone-versus-projection concurrency regression is still needed with the fix.
- Persistence, transactions, and writes: none are modified; the write path was inspected only to validate bucket-hash equivalence.
This was a review-only environment, so I did not run builds or tests. There was no additional user-provided review focus.
Overall: changes requested until the shared-expression concurrency race is removed.
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Late runtime-filter conjuncts became visible to scanner cloning before partition projection completed. VExprContext::clone() opens the shared expression root while projection executes it, so releasing the conjunct lock before pruning allowed concurrent mutation and reads of expression node state. Keep the conjunct lock through pruning while continuing to process only the newly appended filter slice, preserving linear work across staggered arrivals. ### Release note None ### Check List (For Author) - Test: Unit Test - BE ScannerLateArrivalRfTest, RuntimeFilterPartitionPrunerTest, and RuntimeFilterBucketPrunerTest (18 tests) - BE clang-format and format check - BE clang-tidy on modified C++ files - Behavior changed: No - Does this need documentation: No
|
/review |
There was a problem hiding this comment.
Review status: complete after two full-scope convergence rounds. Requesting changes for the P2 performance issue described inline.
Checkpoint conclusions:
- Goal and correctness: The PR conservatively uses exact IN runtime filters to skip tablets for direct single-column HASH targets. I independently checked load-time versus pruning CRCs for the admitted physical types and NULLs, exact-to-Bloom fallback, grouped and ungrouped target identity, selected rollups, per-partition bucket counts, and empty sets. No surviving row-loss or query-correctness issue was found.
- Scope and clarity: The patch is focused on FE eligibility and range metadata, optional protocol fields, BE local-state pruning, scheduler checks, counters, and tests.
- Concurrency and lifecycle: Initial and late publication, expression cloning/projection, helper and pruner locks, scheduler checks, early EOS, errors, and teardown were traced. The current head keeps projection serialized with cloning, the pruned sets are monotonic, and no deadlock, race, stale unsafe publication, or lifetime issue survived.
- Configuration and compatibility: The query-scoped switch is forwarded consistently. Appended optional Thrift fields make old-FE/new-BE and new-FE/old-BE combinations fall back conservatively when metadata is absent.
- Parallel and special paths: Ordinary, colocate, bucket-shuffle, Nereids, sampled/specified, query-cache HIT/INCREMENTAL, point-query, disabled, composite, casted, Bloom, and varying-bucket-count paths were checked and preserve provenance or disable the optimization.
- Errors and observability: New status paths are checked or conservatively continue scanning, malformed partial range metadata fails loudly, and BucketsPrunedByRuntimeFilter exposes activation.
- Tests and results: The FE and BE unit tests plus regression cover basic eligibility/fallback, result correctness, NULL, empty/non-exact sets, different bucket counts, composite distribution, disablement, and the profile counter. Late multi-scanner and non-INT end-to-end cases remain coverage gaps, but no distinct defect was found at those boundaries. I did not run builds or tests because this review runner prohibits them.
- Performance: The remaining issue is repeated exact-set materialization and CRC computation across pipeline local states, described inline. Earlier within-state accumulated-filter and ineligible-lock issues are fixed at the current head.
- Transactions, persistence, writes, and security: Not applicable; this is query-local read pruning and does not change stored data, transaction state, edit logs, authorization, or a security boundary.
- User focus: No additional review focus was provided, so the full PR was reviewed.
Overall: one P2 performance issue remains; no additional blocking correctness issue was substantiated.
morrySnow
left a comment
There was a problem hiding this comment.
Reviewed the runtime-filter bucket pruning feature end-to-end. I verified the hash semantics (pruner's update_crcs_with_value reproduces the write-path distribution hash RawValue::zlib_crc32 used in VOlapTablePartitionParam::find_tablets, including the zlib_crc_hash_null null handling), the FE classification guards (cast-wrapped targets rejected), and the producer-side disabled-filter handling (no conjunct appended when the IN set is cleared). Found the issues below as inline comments.
TPC-H: Total hot run time: 28974 ms |
TPC-DS: Total hot run time: 158391 ms |
ClickBench: Total hot run time: 25.11 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Every scan local state materialized and hashed the same finalized exact runtime-filter set independently, point-query scans could be classified for bucket pruning without the metadata required during serialization, and bucket tests derived expectations through the same hashing API as production pruning. Cache immutable bucket hashes once per finalized wrapper and target nullability, reject point-query targets during classification, and validate routing hashes through the write path plus an end-to-end nullable null-aware join. ### Release note None ### Check List (For Author) - Test: Unit Test and Regression test - BE RuntimeFilterBucketPrunerTest (7 tests) - FE RuntimeFilterBucketPruneClassifierTest (11 tests) - Regression test query_p0/runtime_filter/rf_bucket_pruning, generated and verified against $run_path/doris - Release build: ./build.sh --be --fe -j 48 - BE clang-format, format check, and clang-tidy on modified C++ files - Behavior changed: No - Does this need documentation: No
|
/review |
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime-filter bucket pruning repeated metadata attachment and retained redundant per-tablet state, while late filters could arrive during scanner preparation and still allow reader initialization. Enforce finalized hash-cache usage, document the intentional temporary column materialization, attach FE bucket metadata once, retain compact selected-bucket state, and recheck late filters before scanner open. Also make the concurrent scanner test cleanup unconditional. ### Release note None ### Check List (For Author) - Test: Unit Test - BE: RuntimeFilterBucketPrunerTest and ScannerLateArrivalRfTest (12 tests) - FE: ThriftPlansBuilderTest (4 tests) - BE clang-format/check-format and clang-tidy - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Parallel scanners dropped bucket identity, multiple ready filters overcounted pruned tablets, and scanners pruned after prepare retained reader resources until query teardown. Preserve bucket metadata in the parallel scanner factory, count each newly pruned tablet once, and release prepared-but-unopened scanner resources immediately. ### Release note Fix runtime-filter bucket pruning for parallel scanners and release resources for scanners pruned before open. ### Check List (For Author) - Test: Unit Test - BE unit tests: RuntimeFilterBucketPrunerTest.* and ScannerLateArrivalRfTest.* (15 tests) - Format: build-support/check-format.sh - Static analysis: build-support/run-clang-tidy.sh on modified C++ files - Behavior changed: Yes. Late runtime filters can prune parallel scanners and prepared scanners release resources before open. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Bucket pruning could leave constructor-owned OLAP reader inputs attached when bounded concurrency delays a scanner until after a late filter arrives. The FE classifier could also accept a computed MV column when its alias and type collided with the base distribution column, and the parallel scanner builder duplicated bucket identity in a node-based map for every range. Release unopened scanner resources before or after prepare, require direct base SlotRef identity for MV columns, and read bucket identity from aligned owned scan ranges without a duplicate map. ### Release note None ### Check List (For Author) - Test: Unit Test - FE: RuntimeFilterBucketPruneClassifierTest (12 tests) - BE: RuntimeFilterBucketPrunerTest and ScannerLateArrivalRfTest (17 tests) - Formatting: build-support/check-format.sh - Static analysis: build-support/run-clang-tidy.sh on modified C++ files - Behavior changed: Yes. Unsafe computed MV bucket pruning is disabled and unopened pruned scanners release resources promptly. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime-filter partition and bucket pruning eligibility was generated during legacy translation and re-gated in multiple Thrift paths, while bucket range metadata was attached from a coordinator-wide pre-pass. This split ownership across planning stages, repeated session checks, and made missing optional bucket metadata abort planning. Generate both pruning decisions once when Nereids creates the filter at its final target scan, carry immutable target metadata through translation, attach bucket fields idempotently from OlapScanNode.toThrift(), and conservatively disable bucket pruning for the scan when range metadata is incomplete. ### Release note None ### Check List (For Author) - Test: Unit Test - FE: RuntimeFilterPruneClassifierTest, RuntimeFilterTranslatorBucketPruneTest, PhysicalPlanTranslatorTest#testRfPartitionPruneSnapshotSurvivesEnablementChange, OlapScanNodeTest, ThriftPlansBuilderTest - FE build: ./build.sh --fe -j 48 - Behavior changed: Yes. Pruning eligibility and session gating are generated once in Nereids; incomplete optional bucket metadata disables only this optimization instead of failing planning. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime-filter bucket pruning treated a target column without a define expression as a base column. When a non-base index is selected and the target column provenance cannot be proven, a computed materialized-view column could therefore be classified as the HASH distribution column and cause incorrect tablet pruning. Reject only non-base-index targets without a direct base-column definition, while preserving base-index targets and direct SlotRef rollup targets. ### Release note Prevent runtime-filter bucket pruning when a selected non-base index target has unknown base-column provenance. ### Check List (For Author) - Test: Unit Test - RuntimeFilterPruneClassifierTest - Behavior changed: Yes. Bucket pruning now falls back when a non-base-index target has no direct base-column definition. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Runtime-filter partition pruning treated matching Column unique IDs as proof that a target was a partition column. Column unique IDs are allocated independently for base and rollup indexes, so unrelated rollup and partition columns can collide and produce invalid pruning metadata. Resolve a selected-index target to a proven direct base column, compare its name and type with the serialized partition column, and conservatively reject missing, computed, or renamed rollup definitions. ### Release note Prevent runtime-filter partition pruning from using unrelated rollup columns with colliding column IDs. ### Check List (For Author) - Test: Unit Test - RuntimeFilterPruneClassifierTest - Behavior changed: Yes. Partition pruning now requires a target with proven base-column provenance and a directly serializable partition-column identity. - Does this need documentation: No
a9cff25 to
5968e72
Compare
|
run buildall |
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: OlapScanner no longer owns _slot_id_to_virtual_column_expr after the scan-state refactor, but release_unopened_resources still referenced the removed member. This caused BE compilation to fail. Remove the stale cleanup call while retaining cleanup of scanner-owned virtual column expressions.
### Release note
None
### Check List (For Author)
- Test: Manual static check
- clang-format-16 --dry-run --Werror and git diff --check
- Behavior changed: No
- Does this need documentation: No
|
run buildall |
TPC-H: Total hot run time: 16999 ms |
TPC-DS: Total hot run time: 83525 ms |
ClickBench: Total hot run time: 16.63 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#65837 Problem Summary: Recent scan projection and bucket metadata refactors changed the ScannerContext constructor, row descriptor accessor, and OlapScanNode bucket metadata field. The bucket pruning tests still used the old APIs after rebasing, which caused BE test compilation failures and an FE reflection failure. Align the tests with the current production APIs and packed bucket metadata representation. ### Release note None ### Check List (For Author) - Test: Unit Test - BE ASAN UT build compiled scanner_late_arrival_rf_test.cpp successfully; final local link was blocked by an unrelated duplicate getrandom symbol in the local toolchain compatibility library - FE UT checkstyle and test-source compilation passed; local execution was blocked by the JDK Byte Buddy self-attach restriction - Behavior changed: No - Does this need documentation: No
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 16815 ms |
TPC-DS: Total hot run time: 82959 ms |
ClickBench: Total hot run time: 16.43 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by at least one committer and no changes requested. |
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Runtime filters could prune table partitions but still created and scheduled scanners for every hash-distributed bucket. Add FE eligibility metadata for direct targets on a single HASH distribution column and use exact IN values in BE to compute Doris CRC bucket indexes. Initial and late filters skip nonmatching tablet scanners. Composite distribution and non-invertible Bloom filters conservatively fall back.
Release note
Support runtime-filter bucket pruning for exact filters on single-column HASH-distributed OLAP scans. It can be disabled with enable_runtime_filter_bucket_prune.
Check List (For Author)