[improvement](hive) Support partition-column-value-only pushdown for Hive/Hudi scan - #68452
Open
liutang123 wants to merge 19 commits into
Open
liutang123 wants to merge 19 commits into
liutang123 wants to merge 19 commits into
Conversation
liutang123
requested review from
924060929,
CalvinKirs and
morrySnow
as code owners
September 23, 2026 13:21
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes: nine P1 and two P2 issues are detailed inline. The current patch can change query results for empty, deleted, and volatile inputs; it misses default and parallel execution paths; and it leaves both the BE build and connector SPI policy broken.
Critical checkpoint conclusions:
- Goal and proof: Not achieved end to end. Default FileScannerV2 does not use the shortcut, Hudi cannot acquire the capability, and no test demonstrates the intended path.
- Scope and focus: The separate RuntimeFilterPruner changes broaden the PR and include the RANK/DENSE_RANK issue below. No additional user focus was supplied.
- Concurrency: No independent lock race was found, but statement-scope scan reuse omits the mode that changes split shape, so planning order between aliases can select the wrong cached result.
- Lifecycle and static state: No new static-initialization or ownership leak was found. The read lifecycle is incorrect for transactional Hive because the shortcut runs before delete-delta handling.
- Configuration: The new session variable is visible and dynamically read, but defaults on while the default V2 scanner path is unsupported.
- Compatibility: The Thrift value is appended and unsupported readers otherwise fall back to normal scans, but an exhaustive BE switch is not updated and the public connector SPI baseline/API major are unchanged.
- Parallel paths: FileScannerV2, the large-partition batch request, and delegated Hudi capability routing are not updated consistently.
- Conditions and invariants: Eligibility incorrectly treats range existence as row existence, admits ACID tables, ignores volatile/NoneMovable evaluation cardinality, and treats peer-preserving RANK/DENSE_RANK as bounded.
- Tests and results: No test file changed, and the PR reports that FE/BE were not built. The planner rule, wire value, V1/V2 readers, empty/ACID negatives, split reuse/batch modes, SPI surface, and RF pruning are unproved.
- Observability: Existing scan logging and profiles are sufficient to trace planning; no separate instrumentation defect was found, but absent execution evidence leaves the new path unverified.
- Transactions and persistence: This is a read-only change with no EditLog mutation; full-ACID delete deltas are nevertheless bypassed.
- Data writes and crash safety: No write path changes.
- FE-BE variable passing: Translator and Thrift propagation exist, but the default V2 consumer and batch planning request are incomplete.
- Performance: Default V2 can read unsplit whole files, reuse can select the wrong granularity, batch mode creates duplicate ranges, and the RF change can retain a full-domain filter.
- Memory and nullability: No independent allocation or nullability defect was found; explicit partition null markers use the existing filler contract.
- Other error handling: Catching Throwable converts fatal VM/linkage and invariant failures into a normal optimization miss.
Validation was static only, as required by this review runner; no builds or tests were run.
Contributor
There was a problem hiding this comment.
Request changes. A live exact-head review (5293038630) landed during this review with 11 inline comments covering every substantiated finding. I treated those comments as hard duplicate fences and am not reposting substantially similar inline issues.
Checkpoint conclusions:
- Goal and semantics: the partition-only shortcut is not correct or complete end to end. Existing threads cover volatile/non-movable expression cardinality, visible-row and full-ACID violations, and the pure-GROUP-BY retained-project eligibility gap.
- Control flow and parallel paths: FE eligibility, translation, synchronous/batch planning, statement reuse, V1/V2 scanner selection, and base-scanner final conjunct handling were traced. Existing threads cover default V2, batch propagation, reuse identity, and delegated Hudi routing.
- Data lifecycle: exact-head threads cover positive-byte empty files, tolerated missing files, and full-ACID delete deltas. No write or persistence path changes are in scope.
- Configuration and compatibility: the optimization is default-on. Existing threads cover the frozen connector SPI/API-major gate and the exhaustive BE enum switch. Current old-BE consumers otherwise fall back to an ordinary scan, so no separate mixed-version correctness defect was substantiated.
- Concurrency and state: no independent shared-state race survived review. RuntimeFilterPruner state is query-scoped; an existing live thread covers the RANK/DENSE_RANK peer-tie defect.
- Conditions and error handling: existing threads cover broad
catch (Throwable), non-movable evaluation behavior, and scan-range-versus-visible-row invariants. - Tests and observability: no test file changed. Required coverage includes planner eligibility, forced V1/default V2, empty/missing/full-ACID inputs, batch/reuse ordering, Hudi routing, connector SPI, and RuntimeFilterPruner tie behavior.
- User focus: no additional focus points were provided.
Validation was static only, as required by the review task: I did not run builds or tests. The PR description also reports no tests run; compile/API-gate conclusions in the existing review are based on direct static inspection, not executed validation. No additional inline comment is included because the live exact-head threads already cover all accepted findings.
924060929
previously approved these changes
Sep 24, 2026
liutang123
force-pushed
the
opt-partition-scan-master
branch
from
October 1, 2026 04:11
b43797d to
32334ac
Compare
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
COMPLETE static review of PR #68452 at head 32334ac6d3fbe2b1445e4508fdd5b605d400bb0e. I reviewed all 31 changed files, their relevant callers and readers, all existing inline threads, and three convergence rounds. Three new issues are attached inline (two P1, one P2). No user-specific focus was supplied. Previously reported P1 issues were independently checked as addressed on this head, so no existing blocking comment is carried forward. No build or test was run under the review contract.
Required checkpoint conclusions:
- Goal and proof: The change aims to answer partition-only Hive aggregates from metadata and refine runtime-filter pruning. The guarded V1/V2 reader and planner paths generally support that goal, but the retained-filter fallback loses splitting (P2), and the new regression fixture cannot exercise its intended null-partition data (P1). Added tests were inspected only; their results were not independently observed.
- Scope and clarity: The FE, connector, SPI, Thrift, BE, and test edits are related to those features. The two new test-source import placements break the FE style gate (P1); no separate unrelated change was identified.
- Concurrency and locks: Checked statement-scoped Hive split reuse, normal/batch planning, per-run runtime-filter state, and BE reader ownership. No new shared mutable state, lock-order problem, or heavy work under a new lock was substantiated.
- Lifecycle and static initialization: Checked V1/V2 scanner setup, EOF, fallback, and close paths, plus CTE producer/consumer traversal. No new cross-translation-unit static initialization dependency or separate lifecycle failure was substantiated.
- Configuration: The new optimization session variable is read when the plan is built, and regular and batch scan requests transport its selected mode. The default-enabled filtered plan still causes the P2 performance regression.
- Compatibility: The connector API version and surface baseline were updated, and the Thrift enum was appended with BE dispatch covered. Older/unsupported readers can take ordinary scan fallback; that path can share the P2 whole-file cost. No distinct confirmed rolling-upgrade failure remained.
- Parallel paths: Checked V1 and V2, Parquet and ORC, normal and batched Hive planning, sampled scans, Hudi delegation, and transactional-table exclusion. The filtered shortcut mismatch is the one confirmed parallel-path issue.
- Conditional guards: Inspected partition-slot, volatile-expression, sampling, no-predicate/delete, selected-footer-range, and TopN bound guards. The retained scan conjunct blocks both BE shortcuts after Hive has already disabled file splitting (P2); no separate wrong-result case was substantiated.
- Test coverage and results: The new BE/FE tests and Hive regression assertions were read, not run. The Hive fixture has two combined JDBC statements (P1), and FE test imports violate Checkstyle at validate (P1). Result and EXPLAIN assertions do not prove the filtered shortcut avoids data reads or retains parallel splits.
- Observability: Existing EXPLAIN mode and scan metrics help trace the path, but the added tests have no read/split counter assertion to expose the P2 fallback. No separate production metric defect was substantiated.
- Transactions, persistence, writes, and crash recovery: Product changes are read/plan paths and add no EditLog or data-write protocol. Transactional Hive is excluded from the shortcut. The regression fixture writes only test tables; its JDBC statement issue is reported inline. No separate persistence or crash issue was found.
- FE–BE transport: The new operation passes through the FE scan request, Thrift enum, and both BE reader paths, including batch mode. The retained filter reaches both BE guards, explaining the P2 fallback.
- Performance and remaining issues: Whole-file serial reads for filtered partition aggregates are the confirmed performance defect. All other candidates were either merged into the three comments or dismissed with code evidence; no suspicious point remains unresolved.
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
Static review complete for head 61c55d34acb73436a204d9674bce9ca77df2035a. I found one new P2 performance issue in CTE runtime-filter pruning (inline). All previously reported P1 issues were rechecked against this head and are resolved; no earlier P0/P1 comment ID needs carrying forward. The existing P2 threads remain covered and are not reposted. There was no additional user review focus.
- Goal and behavior: The change plans partition-value MIN/MAX and grouping for eligible Hive scans, translates
PARTITION_VALUEthrough Thrift, and uses footer-backed nonempty checks in both BE scanner paths. The implementation reaches the intended paths under its stated guards, subject to the inline runtime-filter issue. - Scope and design: I traced the FE rewrite, connector capability and split planning, V1/V2 readers, and runtime-filter postprocessing. The cross-module changes are related to the feature. CTE source inheritance is broader than the producer key semantics support, causing the one finding below.
- Concurrency and lifecycle: No new worker or lock was added. Scan reuse uses the existing concurrent statement cache, and CTE effectiveness state belongs to a fresh postprocessor instance. Reader initialization, synthetic-row production, EOF, fallback, and close were traced; no separate lifetime or ownership defect was substantiated.
- Configuration and compatibility: The new session switch is read while planning each statement. The FE-to-BE enum translation, BE enum switch, connector SPI surface baseline, and API version 13.0 were checked. No distinct protocol failure was substantiated; mixed-version execution was not run.
- Parallel paths and conditions: V1 and default V2 paths, split ranges, Parquet/ORC, empty or missing files, residual predicates, runtime filters, deletes, TABLESAMPLE, volatile expressions, and unsupported/transactional/delegated tables were reviewed. The guarded cases retain ordinary scan semantics and Hive keeps normal split sizing on fallback.
- Tests and results: BE/FE/connector unit tests and the Hive regression changes were inspected, including negative and empty-file cases. No test or build was run in this review environment, so these are coverage observations, not execution results. There is no new result file. A two-consumer CTE plus left-join test is needed for the inline issue; mixed-version runtime behavior and actual I/O savings remain unverified here.
- Observability, errors, and memory: Existing scanner/profile reporting remains available; no separate observability gap or new tracked-allocation issue was substantiated. The reviewed missing/corrupt-file and reader-error paths retain failure/fallback behavior rather than silently inventing a row.
- Transactions, persistence, and writes: This is a read-only scan/planner feature. Transactional Hive is excluded, and the patch does not alter Doris data writes, EditLog, visible-version handling, or delete-bitmap persistence.
- Performance and remaining issues: The accepted CTE case retains a runtime filter that rejects no rows and adds build/transfer work; it can also prevent the metadata shortcut. No other distinct correctness or significant performance issue survived three review rounds and the final 29-file sweep.
The PR description still mentions Hudi support and older test coverage; please update it to match this head.
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 28068 ms |
Contributor
TPC-DS: Total hot run time: 152441 ms |
Contributor
ClickBench: Total hot run time: 24.08 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
liutang123
force-pushed
the
opt-partition-scan-master
branch
from
October 2, 2026 12:16
7159421 to
b945d1c
Compare
added 2 commits
October 2, 2026 23:32
…Hive/Hudi scan ### What problem does this PR solve? Problem Summary: `SELECT max(dt) FROM hive_tbl` (and the `dt = (SELECT max(dt) FROM ...)` latest-partition pattern) currently scans the whole table although the answer is already in the partition metadata. Add PARTITION_VALUE pushdown: the scan emits one row of partition column values per data file without opening any file. Changes, in order: 1. gensrc: add `TPushAggOp.PARTITION_VALUE`. 2. BE: add `PartitionColumnReader` and install it in `FileScanner` when every requested column is a partition column. 3. FE: add session variable `enable_partition_column_value_only_optimization` (default true). 4. FE: add the two `AggregateStrategies` rules plus the PARTITION_VALUE helpers, including the two rules that cross a `LogicalFilter`. 5. FE: treat a CTE producer/consumer, `PartitionTopN` and a no-group-by aggregate as effective runtime-filter sources, so the runtime filter is not pruned away. 6. FE: add `ConnectorCapability.SUPPORTS_PARTITION_VALUE_ONLY` (HIVE/HUDI only) and plumb `ConnectorScanRequest.partitionValuePushdown`, so Hive stops splitting files for this scan. ### Release note New session variable `enable_partition_column_value_only_optimization` (default true): a min/max aggregation over only Hive/Hudi partition columns is answered from partition metadata without reading data files. ### Check List (For Author) - Test: No need to test (with reason) — not run yet: the local FE build fails on a pre-existing thrift version mismatch (thrift 0.16.0 vs libthrift 0.24.0) and BE was not compiled. Regression tests still to be added. - Behavior changed: Yes — see the release note; gated by the new session variable. - Does this need documentation: No
… pushdown ### What problem does this PR solve? Problem Summary: The first version of this optimization treated "the scan range exists" as "the source contributes a row": it opened no file and always emitted one partition row. A file with a nonzero header but zero rows, or a transactional Hive base whose rows are all deleted by delete deltas, therefore made MAX/GROUP BY/DISTINCT return a partition that a normal scan never yields. The default FileScannerV2 path did not participate at all, so with enable_file_scanner_v2=true (the default) the feature was inactive while the connector had already stopped splitting files. Changes, in order: 1. BE: replace the unconditional synthetic reader with a decorator that emits one partition row only after the real Parquet/ORC metadata proves the range is nonempty; unsupported formats, transactional tables, deletes, pending runtime filters and unproven counts fall back to a normal scan. 2. BE: add PARTITION_VALUE to FileScannerV2 aggregate pushdown, reuse the metadata count request, and add the missing generated-enum switch case. 3. Remove the nonexistent compile_check header include (4.1-only) from the new BE header. 4. FE: centralize one eligibility check for every PARTITION_VALUE entry point, rejecting volatile/NoneMovable aggregate, project and filter expressions and TABLESAMPLE; drop the Throwable catch; read partition columns at the scan reference snapshot, locale-independently. 5. FE: mark a PartitionTopN an effective runtime-filter source only for a real row bound (ROW_NUMBER without partition keys, or a global limit), and inherit child effectiveness instead of claiming one; keep CTE producer->consumer inheritance. 6. Connector: grant the capability only to nontransactional Hive Parquet/ORC, remove the unreachable Hudi claim, include the effective split size in the statement reuse key, and forward the mode on the batch split path. 7. Tests: FE unit tests for the rule and the pruner, connector tests for capability/reuse/ batch, BE tests over real Parquet/ORC footers, and a Hive regression comparing every query against an optimization-off baseline across V1/V2. ### Release note Session variable enable_partition_column_value_only_optimization now applies to nontransactional Hive Parquet/ORC tables only, and yields a partition value only for ranges whose file metadata proves at least one row; it is effective on the default FileScannerV2 path as well. ### Check List (For Author) - Test: Regression test / Unit Test. FE unit and connector tests pass (PhysicalStorageLayerAggregateTest 19, RuntimeFilterTest targeted 3, SPI 11, Hive 56). BE tests could not be run: run-be-ut.sh fails configuring contrib/openblas (pre-existing, unrelated), and build.sh --fe stops at "Thirdparty libraries need to be build" and its cleanup of thirdparty/installed needs confirmation. The Hive regression requires the docker Hive environment (enableHiveTest=false locally) and was not executed. - Behavior changed: Yes — see the release note. - Does this need documentation: No
### What problem does this PR solve? Problem Summary: The Hive regression fixture built its null partition with one hive_docker call holding both "set hive.exec.dynamic.partition.mode=nonstrict;" and the INSERT. hive_docker passes the whole string to a single PreparedStatement.execute() (Suite.groovy:1780-1788 -> JdbcUtils:46-53) and only strips a trailing semicolon, so Hive received one invalid two-command string: the SET never applied and the INSERT never ran. The ORC fixture repeated it, and its two-column dynamic insert additionally requires nonstrict mode, so it could not have succeeded either. The fixture therefore silently lacked the null partition value that this suite exists to cover. Because the baseline and the optimized runs read the same reduced data, every assertEquals below still passed - the suite proved nothing about null or empty partition handling. Changes: 1. Issue the SET and each INSERT as separate hive_docker calls; the Hive connection is thread-local and reused (SuiteContext.groovy:287-299), so the setting carries over. 2. Assert the fixture itself before querying Doris: p=4 must hold the null partition value and p=9 must be empty, for both the Parquet and the ORC table. A silently skipped insert now fails loudly instead of making the comparisons vacuous. ### Release note None ### Check List (For Author) - Test: Regression test. Not executed locally: it requires the docker Hive environment (enableHiveTest=false). Only the test file changed. - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Problem Summary: Sparked by review: the connector stopped splitting files whenever the plan carried the PARTITION_VALUE hint, but whether a reader actually takes the reduced path depends on conditions the connector cannot see. A retained filter (SELECT MAX(p) FROM t WHERE p >= 2) becomes scan conjuncts, and V1 requires _conjuncts.empty() while V2 requires the same; a runtime filter that has not arrived fails V1 as well. In every such case the reader falls back to an ordinary scan, yet the file had already been planned as one range, so a large surviving file was read serially by a single scanner instead of the split count a normal scan uses -- slower than the baseline, not merely without gain. Result and EXPLAIN assertions cannot see this, because the plan still reports PARTITION_VALUE in the declined case. Changes, in order: 1. Drop the partitionValuePushdown hint: ConnectorScanRequest, both PluginDrivenScanNode request builders, the Hive connector's split sizing and its reuse key. Splitting is now planned exactly as for any other scan, so a declined reader costs nothing extra. 2. Keep V1's whole-range requirement and document why: a partial range's row count is only dependable when the Parquet reader filters row groups by range, and ORC's count reads 0 until its row reader exists, which would drop a partition instead of duplicating a row. FileScannerV2 has no such requirement, so the default path still benefits per split. 3. Update the connector tests: batch planning now asserts ordinary splits, and the two reuse-isolation cases collapse into one, since differing split shapes no longer occur. ### Release note None ### Check List (For Author) - Test: Unit Test. Connector and SPI suites pass (76 tests, BUILD SUCCESS). The Hive regression is unchanged and still requires the docker environment (not executed). BE tests remain blocked by the pre-existing contrib/openblas configuration failure. - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Problem Summary: The CTE producer->consumer inheritance copied whatever effective-source type the producer root carried. NATIVE is not only set for a bounded relation: the join visitor also marks a join NATIVE when its build side is selective, which is a property of one join key. For c = Project(A.k, B.v) -> LeftJoin(A, Limit(1) -> B) the limited build side makes the join visitor mark the producer NATIVE even though a LEFT JOIN preserves every A.k, so the producer output is not bounded. Copying that flag to every consumer of the CTE let a consumer that only keeps B.v live inherit an effectiveness it cannot justify. Used as the build side of another join, it bypassed the statistics-based runtime-filter pruning, so with un-analyzed external tables sharing the full key domain the filter was built, transferred and evaluated per row while rejecting no row. Changes: 1. Record a producer in the CTE map only when its effectiveness is NATIVE and its plan root has a genuinely bounded output: Limit, TopN, AssertNumRows, a no-group-by aggregate, or a PartitionTopN with a global limit (or ROW_NUMBER without partition keys). 2. Add a two-consumer test for the reviewed shape: the join is still marked from its build side, but neither the consumer keeping A.k nor the one keeping B.v inherits anything. ### Release note None ### Check List (For Author) - Test: Unit Test. RuntimeFilterTest (4 targeted) and PhysicalStorageLayerAggregateTest (19) pass, BUILD SUCCESS. - Behavior changed: No - Does this need documentation: No
…cers A CTE producer handed its effectiveness to consumers only when its plan root was a bounded operator. The root is almost always a Project or Distribute, so the canonical `WITH m AS (SELECT max(p) AS p FROM t)` shape lost the runtime filter that prunes the probe-side scan, and a producer read through a partition filter lost it too -- which defeats the partition-value optimization this branch adds. Rename hasBoundedOutput to hasRelationGlobalEffectiveness and let it accept an Intersect, a predicate on a visible column, and look through Project/Distribute. Propagate REF unchanged (criterion 4); keep NATIVE gated so a join's build-side selectivity still cannot leak through a CTE. Tests: RuntimeFilterTest 44/44; new CTE cases in the Hive partition-value regression (noStatsRfPrune/query23 keeps two runtime filters it should); shape_check tpcds_sf100 rf_prune/shape 98/98, noStatsRfPrune 97/98 (query24, unrelated to this branch). Co-Authored-By: Claude Code <noreply@anthropic.com>
liutang123
force-pushed
the
opt-partition-scan-master
branch
from
October 2, 2026 15:32
fb77613 to
ea7a6de
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review complete for PR #68452 at ea7a6de. I covered all 30 changed files, three normal/risk convergence rounds, the existing threads, and a final changed-file sweep. Two new P2 runtime-filter performance findings are inline. The previously reported P0/P1 cases have targeted fixes at this head, so no existing blocking comment ID is carried. No additional user focus was supplied.
Critical checkpoints:
- Goal and proof: the change reduces partition-only Hive aggregation to one row per proven-nonempty Parquet/ORC range and retains useful CTE runtime filters. BE, FE, connector, and Hive regression tests were added, but they do not cover the two bounded grouped/window CTE shapes reported below.
- Size and clarity: the FE rule, Hive capability, SPI/Thrift mode, and V1/V2 reader implementations are scoped to this feature; the CTE pruning helper has two missed row-preserving paths.
- Concurrency: Hive statement-scope reuse uses its existing concurrent map, reader state is per scan/split, and CTE effectiveness state is per pruner invocation. I found no new shared mutable state, lock order, or heavy work inside a new lock.
- Lifecycle: V1 initializes the real reader before wrapping its footer count; V2 checks each split before materializing a partition row. Empty counts and rejected shortcuts follow ordinary reader/close paths. No new cross-translation-unit static initialization dependency was found.
- Configuration: the new session variable is read during planning and can affect subsequent statements without process restart; no process-wide dynamic setting was added.
- Compatibility: connector API is versioned to 13.0 with its surface baseline; the Thrift enum, FE translator, and current BE switch agree. Mixed-version execution was not exercised.
- Parallel paths: both V1 and default V2 scanner paths, batch and reused Hive splits, and fallback when filters or deletes apply were checked. Normal split sizing remains independent of the optimization hint.
- Conditions: FE restricts the rewrite to partition-only, duplicate-insensitive aggregates and excludes volatile/nonmovable expressions and sampling; BE gates physical format, projection, filters, pending RFs, deletes, and positive row counts. The two new CTE source checks are too narrow for bounded children.
- Tests and expected results: I inspected BE/FE unit cases, the Hive fixture and V1/V2 comparisons, negative cases, and the updated RF shape output. The missing grouped/window CTE cases are called out inline. This was a static review; no build or test was run, so runtime results are unverified.
- Observability: existing scan profiles and EXPLAIN RF assertions expose most changed paths; the missing RF plans need specific EXPLAIN coverage.
- Transactions, persistence, and writes: no storage write, EditLog, or transaction path changed; transactional Hive is excluded from the capability.
- FE-to-BE state: PARTITION_VALUE is translated into the appended Thrift enum and handled by both current scanner implementations.
- Performance and remaining issues: ordinary split sizing protects fallback parallelism; M2 and M3 can still discard selective Hive partition RFs and force extra scan work. No other distinct reachable issue survived deduplication and the final sweep.
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 28215 ms |
Contributor
TPC-DS: Total hot run time: 152423 ms |
Contributor
ClickBench: Total hot run time: 25.1 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…ndows A grouped aggregate never adds rows, so a bound proven on its input still holds on its output; a window keeps exactly one output row per input row. Rejecting both meant a twice-referenced materialized CTE such as `HashAggregate(group=k) -> Limit(1) -> HiveScan` or `Project(p,n) -> Window(...) -> Limit(1) -> HiveScan` handed no effectiveness to either consumer, and a join building an RF for `probe.k = c.k` then dropped it. That hurts external tables most: RuntimeFilter.canPruneScanRanges() only classifies PhysicalOlapScan, so a Hive target has no fallback once the build side is not effective. A join still stops the walk, so a join's build-side selectivity cannot leak through a CTE. Tests: RuntimeFilterTest 46/46 with both two-consumer plans; shape_check tpcds_sf100 rf_prune/shape 98/98, noStatsRfPrune 97/98 (query24, unrelated); Hive partition-value regression passes on hive3. Co-Authored-By: Claude Code <noreply@anthropic.com>
The plan can report pushdown agg=PARTITION_VALUE while the reader declines the range and falls back to an ordinary scan, so asserting on the plan does not prove the optimization took effect. Check the profile instead: with the pushdown the scan feeds the aggregate one row per nonempty range, without it one row per data row (measured 5 vs 8 on this fixture). Also covers PartitionColumnReader::supports_range, which only accepts a whole file range: a split range makes the V1 reader fall back while the plan still claims PARTITION_VALUE. Co-Authored-By: Claude Code <noreply@anthropic.com>
enable_file_scanner_v2=false does not reach scanner V1 for Parquet: FileQueryScanNode stamps parquet_timestamp_semantics_version=1, which FileScanLocalState treats as a required timestamp contract and honours only on V2. So the V1 leg of the existing loop ran on V2 like the other leg. Run the profile check on the ORC fixture instead, where V1 is reachable, and assert UseScannerV2 is false so the case cannot silently drift back to V2. Co-Authored-By: Claude Code <noreply@anthropic.com>
Checkstyle enforces lexicographical order and blank-line-separated groups; the imports added with the window test violated both. Co-Authored-By: Claude Code <noreply@anthropic.com>
Drop the "the footer must prove the range is nonempty" gate: one row of partition values is now emitted for every scan range, so the optimization no longer depends on the reader being able to report a row count. That also lets V1 drop the whole-file requirement, which existed only because a partial range's count was unreliable, and it removes the count request from the V2 path entirely. The trade-off is deliberate: a file that turns out to hold zero rows still yields its partition value, so MAX/GROUP BY/DISTINCT can name a partition a full scan would not return. A partition with no file at all still yields nothing. Grant SUPPORTS_PARTITION_VALUE_ONLY to Hudi as well, and assert on the Hudi counterpart suite that the pushdown is planned and actually ran. Co-Authored-By: Claude Code <noreply@anthropic.com>
… predicate PARTITION_VALUE used to require an empty conjunct list, so a query like `SELECT MAX(p) FROM t WHERE p >= 2` fell back to a full scan even though the predicate only reads partition columns. Scanner::_filter_output_block() already evaluates the scanner's conjuncts on whatever block the table reader returns, so the one-row block synthesized for PARTITION_VALUE is filtered like any other row. The only real requirement is that the conjuncts read nothing but partition columns: the synthesized row carries partition values and nothing else, so a predicate on a data column or on a slot outside the projection would be evaluated against unrelated values. Slotless predicates stay excluded, since they would be evaluated once here instead of once per source row. COUNT and MIN/MAX keep the original blanket guard: their synthetic rows are a reduced image of the whole file, not a real row, so no conjunct may see them. Adds `select max(p) from ... where p<=1` to the Hive suite: the answer is 1, not the unfiltered max of 4, which discriminates this from simply dropping the predicate. Co-Authored-By: Claude Code <noreply@anthropic.com>
…shdown COUNT(DISTINCT p) over a partition column is duplicate-insensitive: the scan emits one row of partition values per file and every row of a file carries the same values, so deduplicating that stream yields the same value set as deduplicating every row. Same reasoning as MIN/MAX, which were already supported. Plain COUNT stays rejected -- it counts rows, and the synthesized stream has one row per file, so COUNT(p) would answer with the file count. Other distinct aggregates stay rejected too. Tests: `count(distinct p)` positive on both fixtures; negatives for `count(p)` and `sum(distinct p)`; and a CTE consumed twice through different shapes (a join and a scalar subquery), which must still inherit the producer's bounded output. Co-Authored-By: Claude Code <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Problem Summary:
SELECT max(dt) FROM hive_tbl(and thedt = (SELECT max(dt) FROM ...)latest-partition pattern) currently scans the whole table although the answer is already in the partition metadata. Add PARTITION_VALUE pushdown: the scan emits one row of partition column values per data file without opening any file.Changes, in order:
TPushAggOp.PARTITION_VALUE.PartitionColumnReaderand install it inFileScannerwhen every requested column is a partition column.enable_partition_column_value_only_optimization(default true).AggregateStrategiesrules plus the PARTITION_VALUE helpers, including the two rules that cross aLogicalFilter.PartitionTopNand a no-group-by aggregate as effective runtime-filter sources, so the runtime filter is not pruned away.ConnectorCapability.SUPPORTS_PARTITION_VALUE_ONLY(HIVE/HUDI only) and plumbConnectorScanRequest.partitionValuePushdown, so Hive stops splitting files for this scan.Release note
New session variable
enable_partition_column_value_only_optimization(default true): a min/max aggregation over only Hive/Hudi partition columns is answered from partition metadata without reading data files.Check List (For Author)
What problem does this PR solve?
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)