[fix](stream) Fix stream scan partition prune state propagation - #65657
Merged
morrySnow merged 2 commits intoJul 23, 2026
Merged
Conversation
yujun777
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
July 15, 2026 10:19
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
yujun777
changed the title
1 task
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28936 ms |
Contributor
TPC-DS: Total hot run time: 177997 ms |
Contributor
ClickBench: Total hot run time: 25.37 s |
Contributor
|
/review |
morrySnow
previously approved these changes
Jul 15, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Found one blocking issue in the stream scan partition-prune state propagation fix.
The changed override now matches the base LogicalOlapScan contract locally, but the corrected hasPartitionPredicate=false state is not preserved through stream normalization. Normalized base/binlog scans can still be reclassified as having manual partition predicates, which affects SQL block-rule enforcement for partitioned stream queries without an effective partition filter.
Review scope and checkpoints: inspected the single changed file plus partition pruning, stream normalization, physical translation, SQL block-rule enforcement, wrapper scan metadata, one-argument selected-partition callers, and existing FE/regression coverage. No concurrency, lifecycle, config, persistence, storage-format, FE-BE protocol, data-write, or transaction-path change was introduced by this PR. User focus file had no additional focus. Validation was static only; this review-only runner is missing thirdparty/installed/bin/protoc, so I did not run FE tests or builds.
yujun777
force-pushed
the
fix-stream-scan-partition-prune-loop
branch
from
July 16, 2026 06:21
f9b4ea9 to
72b7474
Compare
Contributor
Author
|
run buildall |
github-actions
Bot
removed
the
approved
Contributor
TPC-H: Total hot run time: 29271 ms |
Contributor
TPC-H: Total hot run time: 29786 ms |
Contributor
TPC-DS: Total hot run time: 178294 ms |
Contributor
ClickBench: Total hot run time: 25.08 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
morrySnow
approved these changes
Jul 17, 2026
Contributor
|
/review |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
There was a problem hiding this comment.
Request changes: the existing partition-filter propagation blocker remains unresolved at head 72b7474d41f01a654d940c6737b51a43d6fa12f8.
The local builder change is correct: it aligns LogicalOlapTableStreamScan.withSelectedPartitionIds(List<Long>, boolean) with the parent contract, marks both boolean paths as already pruned, and stops the repeated PruneOlapScanPartition rewrite. However, NormalizeOlapTableStreamScan still lowers the stream into fresh base/binlog scans by putting internally selected partition IDs into specifiedPartitions. Recursive top-down pruning then classifies those IDs as a manual partition filter and sets every executable child of a partitioned stream to hasPartitionPredicate=true. An unfiltered partitioned stream query can therefore still bypass a SQL block rule with require_partition_filter=true. This is the same blocker already anchored in the existing inline thread, which GitHub currently marks resolved even though the code path remains; I did not add a duplicate inline comment.
Critical checkpoint conclusions:
- Goal and correctness: the optimizer-loop root cause is fixed locally, but downstream execution semantics remain incorrect because the partition-filter flag is not preserved through normalization.
- Scope and clarity: the two-file patch is small and the production change follows the parent builder contract.
- Parallel paths: incremental, snapshot, and reset lowering all create affected base/binlog scans; both filtered and unfiltered paths were traced through logical implementation, physical translation,
StmtExecutor, andSqlBlockRuleMgr. - Test coverage and results: the new JUnit test deterministically catches the old direct false-value builder behavior, but it stops after analysis and does not exercise the real rewrite loop, the true-value path, normalization, partial empty-partition pruning, final
OlapScanNodestate, or block-rule enforcement. Current PR checks report FE UT, compile, CheckStyle, and regression jobs passing, but they do not disprove these uncovered state transitions. No local build or test was run because this is a review-only runner and the local third-party/protoc dependencies are absent. - Performance: the change removes the unbounded rewrite/allocation loop and adds no hot-path overhead.
- Concurrency, lifecycle, configuration, persistence, transaction/data-write, storage compatibility, FE-BE protocol, memory safety, and observability: not involved in this patch; no issue found for these checkpoints.
- User focus: no additional focus was provided; the full PR and its downstream partition-filter behavior were reviewed.
Please preserve the already-computed hasPartitionPredicate value through both partial empty-partition selection and normalized-child construction. Add end-to-end filtered/unfiltered assertions, including a filtered case where only some selected partitions are empty, before merging.
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Incremental IVM refresh on table stream scans could loop forever during partition pruning. The root cause was `LogicalOlapTableStreamScan.withSelectedPartitionIds(List<Long>, boolean)` redefining the second argument as `isPartitionPruned`, while callers and the parent `LogicalOlapScan` contract treat it as `hasPartitionPredicate`. When `PruneOlapScanPartition` passed `false` for a scan without a partition predicate, the stream scan builder also reset `partitionPruned` to `false`, so the rewrite kept matching the same scan and rebuilding the plan. This caused MTMV tasks such as `mv_ivm_partition_key` incremental refresh to stay running and continuously allocate plans. The fix aligns the stream scan override with the parent contract: the boolean argument now represents `hasPartitionPredicate`, and scans rebuilt through this path are always marked as partition-pruned.
### Release note
None
### Check List (For Author)
- Test: Manual test
- Verified `test_ivm_partition_unique_key` with local regression run: `./run-regression-test.sh --run -d mtmv_p0 -s test_ivm_partition_unique_key`
- Behavior changed: Yes (fixes incremental IVM refresh from hanging during stream scan partition pruning)
- Does this need documentation: No
Add a focused FE UT for LogicalOlapTableStreamScan.withSelectedPartitionIds so the partition-pruned state propagation is covered directly. Key changes: - add a stream scan UT that verifies withSelectedPartitionIds marks partitionPruned - assert hasPartitionPredicate is preserved when pruning state is updated - add a helper to find the first LogicalOlapTableStreamScan from the analyzed plan Unit Test: - run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.ExplainTableStreamPlanTest
yujun777
force-pushed
the
fix-stream-scan-partition-prune-loop
branch
from
July 22, 2026 06:42
72b7474 to
d2bc4f6
Compare
Contributor
Author
|
run buildall |
Contributor
Author
The requested changes are outside the scope of this PR. This patch only fixes the partition-pruned state propagation and aligns
|
Contributor
TPC-H: Total hot run time: 29704 ms |
Contributor
TPC-DS: Total hot run time: 177932 ms |
Contributor
ClickBench: Total hot run time: 25 s |
morrySnow
approved these changes
Jul 22, 2026
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Automated review complete for head d2bc4f6e23518bd80e9338c923aef3448d9cd107. I found no new patch-scoped blocking issue, and this focused builder-contract fix is acceptable.
Critical checkpoint conclusions:
- Goal and correctness: the override now matches
LogicalOlapScan.withSelectedPartitionIds(List<Long>, boolean): the boolean ishasPartitionPredicate, while the rebuilt scan is always markedpartitionPruned=true. The only production two-argument caller computes that predicate flag before dynamic dispatch, and the corrected postcondition removes the stream scan from the nextPruneOlapScanPartitionmatch, terminating the reported rewrite loop. - Scope and clarity: the production change is minimal and copies the remaining scan state unchanged, including partition-prunable predicates, scan parameters, selected tablets/index, preaggregation state, slots, alias, and stream read mode.
- Parallel and conditional paths: I traced both overloads through
PruneOlapScanPartition,PruneEmptyPartition, MV visitors, stream normalization, logical-to-physical conversion,OlapScanNode, and SQL block-rule checking. The one-argument predicate-state and normalized-child concerns are already fully covered by the existing inline discussion, so I did not submit a duplicate. - Test coverage and results: the added JUnit 5 test deterministically checks the corrected local false-value builder contract. It does not execute the real top-down rewrite/normalization path; that exact coverage limitation, including true-predicate and partially-empty cases, is already recorded in the existing thread. This review runner is restricted to review operations, so I ran no build or test and am not claiming local execution results.
- Concurrency and lifecycle: no threads, shared mutable state, locks, static initialization, or resource-lifecycle changes are introduced.
- Configuration, compatibility, and FE/BE contracts: no configuration, persisted format, RPC/thrift field, function symbol, or rolling-upgrade behavior changes.
- Error handling and observability: no new error path, exception boundary, log, or metric is needed for this immutable plan-builder correction.
- Transactions, persistence, and data writes: not involved; there is no EditLog, transaction-state, visibility-version, or data-write change.
- Performance: the fix prevents repeated optimizer rewrites and associated CPU/allocation growth, with no new hot-path work.
- Test result artifacts: no regression
.outfile is involved in this FE unit-test-only change. - User focus: no additional focus was provided; the full two-file patch and its upstream/downstream planner paths were reviewed.
Review convergence completed in one round: two full-review lanes and a separate risk-focused lane all returned NO_NEW_VALUABLE_FINDINGS, and the final changed-file/candidate sweep found no unresolved point or new inline comment.
0AyanamiRei
pushed a commit
to 0AyanamiRei/doris
that referenced
this pull request
Jul 27, 2026
…he#65657) ### What problem does this PR solve? Related Issue: close apache#65654 `LogicalOlapTableStreamScan.withSelectedPartitionIds(List<Long>, boolean)` diverged from `LogicalOlapScan` and changed the meaning of the second parameter. In `LogicalOlapScan`, the second parameter represents `hasPartitionPredicate`, while the rebuilt scan is always marked as partition-pruned. The stream scan override treated that second parameter as `isPartitionPruned`, which let partition pruning rebuild a stream scan that still looked unpruned and could be matched again by the same rewrite rule. This PR keeps the stream-scan override consistent with `LogicalOlapScan`: the second parameter remains `hasPartitionPredicate`, and the rebuilt `LogicalOlapTableStreamScan` is marked as already partition-pruned.
wyxxxcat
pushed a commit
to wyxxxcat/doris
that referenced
this pull request
Aug 17, 2026
…he#65657) ### What problem does this PR solve? Related Issue: close apache#65654 `LogicalOlapTableStreamScan.withSelectedPartitionIds(List<Long>, boolean)` diverged from `LogicalOlapScan` and changed the meaning of the second parameter. In `LogicalOlapScan`, the second parameter represents `hasPartitionPredicate`, while the rebuilt scan is always marked as partition-pruned. The stream scan override treated that second parameter as `isPartitionPruned`, which let partition pruning rebuild a stream scan that still looked unpruned and could be matched again by the same rewrite rule. This PR keeps the stream-scan override consistent with `LogicalOlapScan`: the second parameter remains `hasPartitionPredicate`, and the rebuilt `LogicalOlapTableStreamScan` is marked as already partition-pruned.
69 tasks
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?
Related Issue: close #65654
LogicalOlapTableStreamScan.withSelectedPartitionIds(List<Long>, boolean)diverged fromLogicalOlapScanand changed the meaning of the second parameter. InLogicalOlapScan, the second parameter representshasPartitionPredicate, while the rebuilt scan is always marked as partition-pruned. The stream scan override treated that second parameter asisPartitionPruned, which let partition pruning rebuild a stream scan that still looked unpruned and could be matched again by the same rewrite rule.This PR keeps the stream-scan override consistent with
LogicalOlapScan: the second parameter remainshasPartitionPredicate, and the rebuiltLogicalOlapTableStreamScanis marked as already partition-pruned.