Skip to content

[Enhancement] IVM: re-derive the maintenance query at refresh instead of freezing it (backport #74881)#75129

Merged
wanpengfei-git merged 2 commits into
branch-4.1from
mergify/bp/branch-4.1/pr-74881
Jun 22, 2026
Merged

[Enhancement] IVM: re-derive the maintenance query at refresh instead of freezing it (backport #74881)#75129
wanpengfei-git merged 2 commits into
branch-4.1from
mergify/bp/branch-4.1/pr-74881

Conversation

@mergify

@mergify mergify Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Why I'm doing:

IVM (incremental materialized view) maintenance used to freeze the rewritten maintenance query into a persisted ivmDefineSql at CREATE time and execute that frozen text on every refresh. The rewritten query is the user query plus the hidden group-identity column __ROW_ID__ (from_binary(encode_row_id(keys))) and, for aggregates, the mergeable __AGG_STATE_* columns.

Because the rewrite was frozen, bugfixes to the rewriter could not reach already-created MVs. For example the positional-GROUP BY / SELECT DISTINCT __ROW_ID__ fix in #74493 only helped newly created MVs; existing MVs kept executing their stale, buggy frozen text and produced wrong results until dropped and re-created. The encode scheme was also baked into the SQL literal with no room to evolve.

What I'm doing:

Re-derive the IVM maintenance query from the MV's original viewDefineSql on every refresh (re-running the current, fixed rewriter), instead of executing a frozen string. So rewriter fixes apply retroactively to existing MVs on their next refresh.

Fixes #issue: N/A — internal IVM refresh-mechanism change (no separate tracking issue); makes rewriter fixes such as #74493 apply retroactively to existing MVs.

Mechanism

  • New IvmRefreshDefinition.derive(ctx, mv) / deriveRewrittenQuery(ctx, mv): parse viewDefineSql → analyze → re-run the IVM rewrite with the MV's pinned encodeRowIdVersion (no trial) → analyze again → validate → buildSimple.
  • New IVM MVs stop persisting ivmDefineSql (MaterializedViewAnalyzer / LocalMetastore); both refresh paths re-derive instead. The field is kept Gson-nullable for old metadata.
  • Two correctness gates at refresh, both fail loudly with a "drop & recreate" message instead of silently writing wrong data:
    • row-id strategy gate: the re-derived RowIdStrategy must equal the stored one;
    • shape gate (IvmSchemaCompat): the re-derived output columns (incl. hidden __ROW_ID__/__AGG_STATE) must match the stored MV schema, reusing the CREATE-time column derivation so type equivalence stays identical.
  • Both refresh paths re-derive: the IVM-delta path (MVIVMRefreshProcessor) and the PCT full-rebuild path (MVPCTRefreshProcessor, used for an MV's first refresh / when no delta baseline exists). The IVM-MV predicate is isIncrementalOrAuto() so legacy AUTO-mode IVM MVs are handled too. MVRefreshSchemaChecker (external-base drift) re-derives the same way so its column comparison matches the stored hidden-column schema.
  • For legacy MVs that still carry a frozen ivmDefineSql, a one-time WARN is logged if the re-derived definition differs (advisory only; refresh always uses the re-derived result).

Behavior change — existing IVM MVs created before this change carry a persisted ivmDefineSql; their refresh now executes the re-derived query instead of that frozen text (this is exactly how rewriter fixes become retroactive). New IVM MVs persist no frozen text. PCT (non-IVM) refresh and SHOW CREATE MATERIALIZED VIEW are unchanged. The persisted Task.definition of an IVM MV now displays the user query rather than the rewritten INSERT (display only — the executed plan is always re-derived). On downgrade to a pre-change binary, a new IVM MV (empty ivmDefineSql) falls back to the un-rewritten user query: for aggregate / DISTINCT MVs the old binary's own schema check rejects it with a clear error (base table schema check failed … column count N vs M, refresh fails — verified on a dev cluster), and a scan MV's user query equals its maintenance query so it refreshes correctly. Downgrade therefore fails loudly or stays correct — it does not silently produce wrong data. (Upgrade is smooth: an existing MV with a frozen ivmDefineSql re-derives on its next refresh — also verified on a dev cluster.)

Verification

  • FE unit tests: re-derive round-trip stability across query shapes (scan, GROUP BY col/ordinal, DISTINCT, all-constant, sum/max); retroactivity (frozen buggy text is ignored); strategy/shape gates; schema-checker no-false-drift for INCREMENTAL and AUTO MVs; the PCT first-refresh full-rebuild of an aggregate MV (validated to fail without the fix). Full IVM + MV suites green (239 tests), checkstyle clean.
  • End-to-end on a dev cluster (real iceberg base): aggregate MV with positional GROUP BY 1,2 and SELECT DISTINCT MV both refresh to data that matches the direct query exactly (0 mismatched rows), through both the PCT full-rebuild and the IVM-delta path.

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5

This is an automatic backport of pull request #74881 done by [Mergify](https://mergify.com).

… of freezing it (#74881)

Signed-off-by: Youngwb <yangwenbo_mailbox@163.com>
(cherry picked from commit 14bf004)

# Conflicts:
#	fe/fe-core/src/test/java/com/starrocks/scheduler/mv/ivm/IVMBasedMvRefreshProcessorIcebergTest.java
@mergify

mergify Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 14bf004 has failed:

On branch mergify/bp/branch-4.1/pr-74881
Your branch is up to date with 'origin/branch-4.1'.

You are currently cherry-picking commit 14bf004251.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   fe/fe-core/src/main/java/com/starrocks/catalog/MaterializedView.java
	modified:   fe/fe-core/src/main/java/com/starrocks/scheduler/mv/MVRefreshSchemaChecker.java
	modified:   fe/fe-core/src/main/java/com/starrocks/scheduler/mv/ivm/MVIVMRefreshProcessor.java
	modified:   fe/fe-core/src/main/java/com/starrocks/scheduler/mv/pct/MVPCTRefreshProcessor.java
	modified:   fe/fe-core/src/main/java/com/starrocks/server/LocalMetastore.java
	modified:   fe/fe-core/src/main/java/com/starrocks/sql/analyzer/MaterializedViewAnalyzer.java
	modified:   fe/fe-core/src/main/java/com/starrocks/sql/analyzer/mv/IVMAnalyzer.java
	new file:   fe/fe-core/src/main/java/com/starrocks/sql/analyzer/mv/IvmRefreshDefinition.java
	new file:   fe/fe-core/src/main/java/com/starrocks/sql/analyzer/mv/IvmSchemaCompat.java
	modified:   fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/ivm/common/IvmOpUtils.java
	modified:   fe/fe-core/src/test/java/com/starrocks/scheduler/mv/MVRefreshSchemaCheckerTest.java
	modified:   fe/fe-core/src/test/java/com/starrocks/sql/analyzer/mv/IVMAnalyzerTest.java
	new file:   fe/fe-core/src/test/java/com/starrocks/sql/analyzer/mv/IvmRefreshDefinitionTest.java
	new file:   fe/fe-core/src/test/java/com/starrocks/sql/analyzer/mv/IvmSchemaCompatTest.java

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   fe/fe-core/src/test/java/com/starrocks/scheduler/mv/ivm/IVMBasedMvRefreshProcessorIcebergTest.java

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify

mergify Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@mergify[bot]: Backport conflict, please reslove the conflict and resubmit the pr

@mergify mergify Bot closed this Jun 22, 2026
auto-merge was automatically disabled June 22, 2026 03:27

Pull request was closed

@Youngwb Youngwb reopened this Jun 22, 2026
@wanpengfei-git wanpengfei-git enabled auto-merge (squash) June 22, 2026 03:53
Mergify left conflict markers in IVMBasedMvRefreshProcessorIcebergTest. #74881's
hunk that retargets the getIVMTaskDefinition mock signature sits inside
testImvSourceRangesNotRecordedOnPctFallback -- part of a TVR/imvSource feature not
present on branch-4.1 (no getImvSourceVersionRange), so the hunk could not apply
and dragged the surrounding main-only methods into the conflict.

Resolve by taking branch-4.1's side: the three main-only testImvSource* methods are
dropped (they would not compile here). #74881's own addition,
testFirstFullRefreshOfAggregateIvmMvRebuildsRewrittenInsert, stays -- its helpers
exist on branch-4.1. The resolved file equals branch-4.1 + that one method.

Signed-off-by: Youngwb <yangwenbo_mailbox@163.com>
@wanpengfei-git wanpengfei-git merged commit 0759003 into branch-4.1 Jun 22, 2026
29 checks passed
@wanpengfei-git wanpengfei-git deleted the mergify/bp/branch-4.1/pr-74881 branch June 22, 2026 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants