Skip to content

Commit 5f93052

Browse files
Youngwbclaude
authored andcommitted
[Enhancement] Reject cross-mode ALTER for materialized view refresh_mode (#72380)
Signed-off-by: Youngwb <yangwenbo_mailbox@163.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit ba99abd)
1 parent 5d68e0b commit 5f93052

2 files changed

Lines changed: 24 additions & 39 deletions

File tree

fe/fe-core/src/main/java/com/starrocks/alter/AlterMVJobExecutor.java

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
import com.starrocks.sql.analyzer.MaterializedViewAnalyzer;
6464
import com.starrocks.sql.analyzer.SemanticException;
6565
import com.starrocks.sql.analyzer.SetStmtAnalyzer;
66-
import com.starrocks.sql.analyzer.mv.IVMAnalyzer;
6766
import com.starrocks.sql.ast.AddColumnsClause;
6867
import com.starrocks.sql.ast.AddMVColumnClause;
6968
import com.starrocks.sql.ast.AlterMaterializedViewStatusClause;
@@ -244,49 +243,29 @@ private void alterMVRefreshMode(Map<String, String> properties,
244243
List<Runnable> appliers,
245244
ConnectContext context) {
246245
String mvRefreshMode = PropertyAnalyzer.analyzeRefreshMode(properties);
247-
// cannot alter original pct based mv to incremental or auto, only support original ivm/pct based mv
248-
MaterializedView.RefreshMode currentRefreshMode =
246+
MaterializedView.RefreshMode targetMode =
249247
MaterializedView.RefreshMode.valueOf(mvRefreshMode.toUpperCase(Locale.ROOT));
250-
if (currentRefreshMode.isIncrementalOrAuto()) {
251-
ParseNode mvDefinedQueryParseNode = materializedView.getDefineQueryParseNode();
252-
if ((mvDefinedQueryParseNode instanceof QueryStatement queryStatement)) {
253-
IVMAnalyzer ivmAnalyzer = new IVMAnalyzer(context, null, queryStatement);
254-
255-
Optional<IVMAnalyzer.IVMAnalyzeResult> result;
256-
try {
257-
result = ivmAnalyzer.rewrite(
258-
MaterializedView.RefreshMode.valueOf(mvRefreshMode.toUpperCase(Locale.ROOT)));
259-
} catch (SemanticException e) {
260-
throw new SemanticException("Cannot alter materialized view refresh mode to %s: %s",
261-
mvRefreshMode, e.getMessage());
262-
}
263-
if (result.isEmpty()) {
264-
throw new SemanticException("Cannot alter materialized view refresh mode to %s," +
265-
" because the materialized view is not eligible for %s refresh mode",
266-
mvRefreshMode, mvRefreshMode);
267-
}
268-
// if materialized's original refresh mode is not auto or ivm, throw exception
269-
if (!materializedView.getCurrentRefreshMode().isIncrementalOrAuto()) {
270-
throw new SemanticException("Cannot alter materialized view refresh mode to %s," +
271-
" only support alter original incremental/auto based materialized view",
272-
mvRefreshMode);
273-
}
274-
currentRefreshMode = result.get().currentRefreshMode();
275-
} else {
276-
throw new SemanticException("Cannot alter materialized view refresh mode to %s", mvRefreshMode);
277-
}
248+
MaterializedView.RefreshMode originalMode = materializedView.getCurrentRefreshMode();
249+
250+
// Cross-mode ALTER is not supported. The recovery path for changing refresh_mode is
251+
// drop-and-recreate. Same-mode ALTER is allowed and behaves as a no-op (modulo the
252+
// applier below, which keeps the persisted property and the in-memory mode in sync).
253+
if (originalMode != targetMode) {
254+
throw new SemanticException(
255+
"Altering refresh_mode from %s to %s is not supported. " +
256+
"Please drop and re-create the materialized view if a mode change is required.",
257+
originalMode, targetMode);
278258
}
279259

280-
MaterializedView.RefreshMode finalCurrentRefreshMode = currentRefreshMode;
281260
// Trigger applier when either the persisted property or the in-memory current mode
282261
// would change. Comparing only the persisted string would miss cases where the two
283262
// are out of sync (e.g. mode promoted internally without rewriting the property).
284263
if (!tableProperty.getMvRefreshMode().equals(mvRefreshMode)
285-
|| materializedView.getCurrentRefreshMode() != finalCurrentRefreshMode) {
264+
|| materializedView.getCurrentRefreshMode() != targetMode) {
286265
appliers.add(() -> {
287266
tableProperty.modifyTableProperties(PropertyAnalyzer.PROPERTIES_MV_REFRESH_MODE, mvRefreshMode);
288267
tableProperty.setMvRefreshMode(mvRefreshMode);
289-
materializedView.setCurrentRefreshMode(finalCurrentRefreshMode);
268+
materializedView.setCurrentRefreshMode(targetMode);
290269
});
291270
}
292271
}

fe/fe-core/src/test/java/com/starrocks/alter/AlterMaterializedViewTest.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,17 @@ public void testChangeMVRefreshMode1() throws Exception {
5252
MaterializedView mv = createMaterializedViewWithRefreshMode(query, "auto");
5353
Assertions.assertEquals(MaterializedView.RefreshMode.AUTO, mv.getCurrentRefreshMode());
5454

55-
// change refresh mode from auto to incremental
55+
// alter auto -> incremental: rejected (cross-mode ALTER not supported)
5656
{
5757
String alterStmt = "alter materialized view test_mv1 set (\"refresh_mode\" = \"incremental\")";
58-
alterMaterializedView(alterStmt, false);
59-
Assertions.assertEquals(MaterializedView.RefreshMode.INCREMENTAL, mv.getCurrentRefreshMode());
58+
alterMaterializedView(alterStmt, true);
59+
Assertions.assertEquals(MaterializedView.RefreshMode.AUTO, mv.getCurrentRefreshMode());
6060
}
61-
// change refresh mode from incremental to auto: rejected (AUTO is hidden from users)
61+
// alter auto -> auto: rejected (AUTO is hidden from users at the analyzer level)
6262
{
6363
String alterStmt = "alter materialized view test_mv1 set (\"refresh_mode\" = \"auto\")";
6464
alterMaterializedView(alterStmt, true);
65-
Assertions.assertEquals(MaterializedView.RefreshMode.INCREMENTAL, mv.getCurrentRefreshMode());
65+
Assertions.assertEquals(MaterializedView.RefreshMode.AUTO, mv.getCurrentRefreshMode());
6666
}
6767
}
6868

@@ -104,6 +104,12 @@ public void testChangeMVRefreshMode3() throws Exception {
104104
alterMaterializedView(alterStmt, true);
105105
Assertions.assertEquals(MaterializedView.RefreshMode.INCREMENTAL, mv.getCurrentRefreshMode());
106106
}
107+
// alter incremental -> pct: rejected (cross-mode ALTER not supported)
108+
{
109+
String alterStmt = "alter materialized view test_mv1 set (\"refresh_mode\" = \"pct\")";
110+
alterMaterializedView(alterStmt, true);
111+
Assertions.assertEquals(MaterializedView.RefreshMode.INCREMENTAL, mv.getCurrentRefreshMode());
112+
}
107113
}
108114

109115
@Test

0 commit comments

Comments
 (0)