Repository navigation
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31635 ms |
Contributor
TPC-DS: Total hot run time: 171215 ms |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run cloud_p0 |
Contributor
FE Regression Coverage ReportIncrement line coverage |
foxtail463
force-pushed
the
fix/nested-mv-relation-implied-predicates
branch
from
May 26, 2026 07:32
d81c332 to
f6264f3
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31602 ms |
Contributor
TPC-DS: Total hot run time: 173393 ms |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
foxtail463
force-pushed
the
fix/nested-mv-relation-implied-predicates
branch
from
June 4, 2026 07:24
f6264f3 to
ad3c922
Compare
foxtail463
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
June 4, 2026 07:24
foxtail463
force-pushed
the
fix/nested-mv-relation-implied-predicates
branch
from
June 15, 2026 10:11
ad3c922 to
08a03f1
Compare
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28962 ms |
Contributor
TPC-DS: Total hot run time: 168704 ms |
Contributor
FE Regression Coverage ReportIncrement line coverage |
foxtail463
force-pushed
the
fix/nested-mv-relation-implied-predicates
branch
from
June 15, 2026 19:15
08a03f1 to
11a1303
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28683 ms |
Contributor
TPC-DS: Total hot run time: 152163 ms |
Contributor
ClickBench: Total hot run time: 24.71 s |
foxtail463
force-pushed
the
fix/nested-mv-relation-implied-predicates
branch
from
October 20, 2026 11:12
1f1d5ed to
aae24cf
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28147 ms |
Contributor
TPC-DS: Total hot run time: 150893 ms |
Contributor
ClickBench: Total hot run time: 24.44 s |
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed all 15 changed paths at aae24cf5c0481e4bc7800ace7544b01cc6eb6821, including upstream analysis, cache publication/invalidation, nested matching and downstream compensation. No new actionable findings. The main review and all three assigned reviewers completed their scopes and final rechecks; the review converged in round 1. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and behavior: candidate-only removal of lower-MV-guaranteed filters supports nested rewriting, including projected-away predicate columns. Query predicates remain available for compensation, and cached definitions remain unchanged. Added unit and data-regression cases exercise these behaviors.
- Scope and reuse: the changes are focused and reuse existing predicate pull-up, implication, cache ownership and locking mechanisms.
- Concurrency: query planning versus refresh/view/schema/drop paths was traced. MV locks protect generation/state/snapshot transitions; table locks remain ID-ordered; expensive cache planning stays outside the MV lock. No new lock inversion was established.
- Lifecycle: immutable facts follow cache/statement ownership. Generation checks reject builds and local entries from earlier planning attempts; partial refresh, failure, replay, IVM and compatibility initialization paths were reviewed. No new resource cycle or initialization hazard found.
- Configuration: no configuration added; the existing zero-sized global-cache path is covered by the generation-aware retry test.
- Compatibility: no persisted format or FE/BE protocol migration introduced; new output facts remain ephemeral.
- Thrift/protobuf: no definitions changed; cross-branch field-ID checks are inapplicable.
- Parallel paths: reviewed full/partial plans, sync/async contexts, guarded/unguarded and global/local caches, live/replay and IVM refresh, plus direct/generated/copied scans.
- Conditions and semantics: base-index and visible-output mapping, volatile/nondeterministic/alias guards, full partition coverage, DNF overflow, projection/aggregation, outer/mark/ASOF joins and NULL semantics conservatively retain filters when proof is unavailable. Candidate mapping exceptions are caught per candidate.
- Test coverage: unit cases cover binding, nested matching, projection, null rejection, hidden IVM columns, aliases/time functions, definition changes, in-flight builds, partial-refresh recovery, disabled-cache retries and stale-upper compensation. The regression additionally compares actual query results.
- Expected results: the ordered before/after rows in the new output file match the fixture. This static review cannot establish generator provenance or a runtime pass.
- Observability: existing cache-generation/derivation and candidate-failure diagnostics cover the new failure paths; no additional metric requirement identified.
- Persistence/failover: existing status/view/task-result journal and replay paths reconstruct snapshot coverage; no new durable expression metadata is required.
- Writes and crash boundaries: no new data-write path. Cache publication was checked against successful/failed refresh and invalidation boundaries.
- FE/BE variables: none added.
- Performance: subtree identity caching and the existing bounded DNF proof budget limit repeated inference. No concrete new performance defect established; runtime performance was not measured.
- Other invariants: no additional supported wrong-result path found. Individual rewrite failures decline candidates; BE visible-version, delete-bitmap and memory-accounting paths are unchanged.
Existing review dispositions:
- Retry finding 4272521191 is fixed: both local-cache lookup sites and publication validate the current MV generation, and refresh/definition invalidation advances it.
- Direct-write findings 4131025029 / 4140246936 describe the same opt-in mutation limitation already affecting ordinary MV transparency. Mutable-binding findings 4131025038 / 4140246940 share the existing untracked function/variable dependency gap, which also affects ordinary expression-output MVs. These physical scenarios are not claimed fixed. Current mutation gates/dependency code and the existing replies support treating them as pre-existing or unsupported transparency limitations, rather than remaining in-scope blockers for this change; no duplicate inline comments are posted.
- The existing mapping-assertion concern is covered by per-candidate catches; the requested volatile-check ordering is present. Analysis-time alias/nondeterminism recording remains necessary before expansion/folding erases dependencies.
No unresolved candidates or changed-path coverage gaps remain. Validation was static only: this review contract prohibits builds, tests and source changes. No runtime correctness or test-pass claim is made.
Review converged after 1 round(s).
morrySnow
approved these changes
Oct 21, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
This was referenced Oct 21, 2026
hello-stephen
pushed a commit
that referenced
this pull request
Oct 21, 2026
Related PR: #69311, #69268, #63639 Problem Summary: #69311 renamed the row binlog (the change stream IVM reads) to **changelog** and rejected the legacy keys `binlog.format` / `binlog.need_historical_value` in every DDL. Two test files merged just before it fell outside that rename sweep and now fail: 1. `regression-test/suites/mtmv_p0/ivm/test_ivm_datetime_sequence.groovy` (from #69268) — creates its IVM base tables with `"binlog.enable" = "true", "binlog.format" = "ROW", "binlog.need_historical_value" = "true"`, which CREATE TABLE now rejects. 2. `RelationImpliedPredicateTest#testIvmOutputPredicatesSkipHiddenColumns` (from #63639) — same legacy properties in the base-table DDL, and it gates the creation on `enable_feature_binlog` instead of the new `enable_feature_changelog`. Fix: switch both to `"changelog.enable" = "true"` (plus `"changelog.need_historical_value" = "true"` where the before-image is needed), drop the rejected `binlog.format`, and use `enable_feature_changelog` as the feature gate in the FE unit test. Related IVM trace issue: #65418 .
yiguolei
pushed a commit
that referenced
this pull request
Oct 29, 2026
#69381) Cherry-picked from #63639 --------- Co-authored-by: yangtao555 <[email protected]>
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
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.
Problem Summary:
Nested MV rewriting can fail when an upper MV repeats a filter that is already guaranteed by its lower MV.
For example:
For the query:
The query can match mv_fact_snapshot_v1, but matching mv_detail_trade_joined_v1 may be rejected because of the repeated t.sku_type = '1' filter. However, mv_fact_snapshot_v1 already guarantees that every output row satisfies this predicate, so the upper-MV filter is redundant.
Solution:
Use reliable predicates guaranteed by the lower MV to prove redundant filters in upper-MV definitions during candidate matching. This allows nested rewriting to match mv_detail_trade_joined_v1 without requiring the upper MV to expose additional filter columns.
Query predicates must still be preserved for compensation. Cached MV definitions remain unchanged. Output-predicate guarantees are withheld whenever the MV definition or refresh state cannot reliably establish them.