[fix](regression) Stabilize sync MV selection in MTMV test#65485
[fix](regression) Stabilize sync MV selection in MTMV test#65485shuke987 wants to merge 1 commit into
Conversation
### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#37396 Problem Summary: The test verifies that an async MTMV can read a sync rollup and resolve its partition column back to the base table. After the old non-cost-based sync MV control was removed, TRY_IN_RBO could successfully generate the rollup candidate while cost-based planning still selected the base index. Add USE_MV to both the precondition query and the MTMV definition so the test deterministically exercises the intended sync-rollup path. ### Release note None ### Check List (For Author) - Test: Groovy parse and git diff --check - Regression test syntax validation - Behavior changed: No - Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Automated review summary for PR #65485:
I reviewed the single-file regression change in regression-test/suites/rollup_p0/test_create_mv_and_mtmv.groovy against the live GitHub patch and relevant FE paths for USE_MV, sync rollup selection, MTMV create analysis, stored query SQL, and MTMV refresh planning. I did not find a substantiated issue that needs an inline review comment.
Critical checkpoint conclusions:
- Goal/test intent: The change keeps the existing MTMV full and partial refresh assertions while forcing the precondition query and stored MTMV query through the intended sync rollup path.
- Scope: The patch is small and focused: one explanatory comment, two
USE_MVhints, and EOF newline cleanup. - Parallel paths: The hint is present both in the precondition explain query and in the MTMV definition. The refresh path reparses
mtmv.getQuerySql()throughUpdateMvByPartitionCommand.from, so the stored hint is available for later refresh planning. - Session/config compatibility: Existing
pre_materialized_view_rewrite_strategy = TRY_IN_RBOremains, and MTMV context creation restores saved result-affecting session variables for refresh. - Test output determinism: Existing
qt_checks order bydt, advertiser; the expected output inregression-test/data/rollup_p0/test_create_mv_and_mtmv.outremains consistent because result rows do not change. - Concurrency/lifecycle/persistence: No new concurrency, lifecycle, persistence format, FE-BE protocol, or configuration item is introduced.
- Validation limits: This runner is a shallow/grafted checkout with missing
.worktree_initialized, missingthirdparty/installed, and nogroovycommand, so I did not run the full regression suite. I verified the live PR patch scope, existing review-comment state, expected output, and added-line whitespace statically.
User focus: No additional user-provided review focus was present.
Subagent conclusions: The optimizer-rewrite and tests-session-config subagents both completed the initial pass with no candidates. The main review dismissed one suspected duplicate-hint-state interaction with code evidence. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger and empty proposed final comment set.
What problem does this PR solve?
Issue Number: None
Related PR: #37396
Problem Summary:
test_create_mv_and_mtmvverifies that an async MTMV can read a sync rollup MV, resolve the rollup column back to the base-table partition column, and complete full and partial refreshes.The original case disabled cost-based sync MV selection so both the precondition query and MTMV definition actually read the sync rollup. After that control was removed,
TRY_IN_RBOcould successfully generate the rollup candidate while cost-based planning still selected the base index, causing an intermittentMaterializedViewRewriteSuccessButNotChosefailure before MTMV creation.Add
USE_MVto both queries so the case deterministically exercises the intended sync-rollup path while retaining the existing full and partial refresh assertions.Release note
None
Check List (For Author)
Test
GroovyShell.git diff --check.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)