This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new e56480ed08e [fix](schema-change) Forbid modifying mv related columns
(#47271)
e56480ed08e is described below
commit e56480ed08e4601d0605d49f9070dffb8939a498
Author: Siyang Tang <tangsiy...@selectdb.com>
AuthorDate: Sat Feb 8 14:38:00 2025 +0800
[fix](schema-change) Forbid modifying mv related columns (#47271)
### What problem does this PR solve?
Problem Summary:
Since lack of ability of materialized views to modify columns with
schema change(with expressions), forbid modifying such columns.
This behavior has been established since nereids support alter table
command #44058.
This PR is for lagecy alter table statement and works for version
without nereids' alter table command.
---
.../apache/doris/alter/SchemaChangeHandler.java | 9 ++-
.../apache/doris/nereids/util/RelationUtil.java | 65 +++++++++++++++++----
.../schema_change_modify_mv_column_type_agg.out | Bin 2634 -> 1367 bytes
.../schema_change_modify_mv_column_type.out | Bin 9551 -> 7289 bytes
.../routine_load_mapping.groovy | 13 +----
.../schema_change_modify_mv_column_type_agg.groovy | 23 +-------
.../schema_change_modify_mv_column_type.groovy | 26 +--------
7 files changed, 66 insertions(+), 70 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
index 51671f4aa29..b27783feece 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
@@ -25,7 +25,6 @@ import org.apache.doris.analysis.CancelAlterTableStmt;
import org.apache.doris.analysis.CancelStmt;
import org.apache.doris.analysis.ColumnPosition;
import org.apache.doris.analysis.CreateIndexClause;
-import org.apache.doris.analysis.CreateMaterializedViewStmt;
import org.apache.doris.analysis.DropColumnClause;
import org.apache.doris.analysis.DropIndexClause;
import org.apache.doris.analysis.Expr;
@@ -718,8 +717,8 @@ public class SchemaChangeHandler extends AlterHandler {
for (Column column : schema) {
String columnName = column.getName();
if (column.isMaterializedViewColumn()) {
- columnName = MaterializedIndexMeta.normalizeName(
-
CreateMaterializedViewStmt.mvColumnBreaker(columnName));
+ throw new DdlException("Can not modify column
contained by mv, mv="
+ + olapTable.getIndexNameById(entry.getKey()));
}
if (columnName.equalsIgnoreCase(modColumn.getName())) {
otherIndexIds.add(entry.getKey());
@@ -735,8 +734,8 @@ public class SchemaChangeHandler extends AlterHandler {
Column col = otherIndexSchema.get(i);
String columnName = col.getName();
if (col.isMaterializedViewColumn()) {
- columnName = MaterializedIndexMeta.normalizeName(
-
CreateMaterializedViewStmt.mvColumnBreaker(columnName));
+ throw new DdlException("Can not modify column
contained by mv, mv="
+ + olapTable.getIndexNameById(otherIndexId));
}
if (!columnName.equalsIgnoreCase(modColumn.getName())) {
continue;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java
index 46c594d4994..0ef9bdd9465 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java
@@ -35,12 +35,14 @@ import
org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.OriginStatement;
+import org.apache.doris.qe.SessionVariable;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
@@ -49,6 +51,13 @@ import java.util.stream.Collectors;
* relation util
*/
public class RelationUtil {
+ private static final String SYNC_MV_PLANER_DISABLE_RULES =
"OLAP_SCAN_PARTITION_PRUNE, PRUNE_EMPTY_PARTITION, "
+ + "ELIMINATE_GROUP_BY_KEY_BY_UNIFORM, HAVING_TO_FILTER,
ELIMINATE_GROUP_BY, SIMPLIFY_AGG_GROUP_BY, "
+ + "MERGE_PERCENTILE_TO_ARRAY, VARIANT_SUB_PATH_PRUNING,
INFER_PREDICATES, INFER_AGG_NOT_NULL, "
+ + "INFER_SET_OPERATOR_DISTINCT, INFER_FILTER_NOT_NULL,
INFER_JOIN_NOT_NULL, MAX_MIN_FILTER_PUSH_DOWN, "
+ + "ELIMINATE_SORT, ELIMINATE_AGGREGATE, ELIMINATE_LIMIT,
ELIMINATE_SEMI_JOIN, ELIMINATE_NOT_NULL, "
+ + "ELIMINATE_JOIN_BY_UK, ELIMINATE_JOIN_BY_FK,
ELIMINATE_GROUP_BY_KEY, ELIMINATE_GROUP_BY_KEY_BY_UNIFORM, "
+ + "ELIMINATE_FILTER_GROUP_BY_KEY";
/**
* get table qualifier
@@ -137,23 +146,55 @@ public class RelationUtil {
Optional<String> querySql = new
NereidsParser().parseForSyncMv(createMvSql);
if (querySql.isPresent()) {
LogicalPlan unboundMvPlan = new
NereidsParser().parseSingle(querySql.get());
- StatementContext statementContext = new
StatementContext(ConnectContext.get(),
+ ConnectContext connectContext = ConnectContext.get();
+ StatementContext statementContext = new
StatementContext(connectContext,
new OriginStatement(querySql.get(), 0));
NereidsPlanner planner = new NereidsPlanner(statementContext);
if (statementContext.getConnectContext().getStatementContext()
== null) {
statementContext.getConnectContext().setStatementContext(statementContext);
}
- planner.planWithLock(unboundMvPlan, PhysicalProperties.ANY,
ExplainCommand.ExplainLevel.REWRITTEN_PLAN);
- LogicalPlan logicalPlan = (LogicalPlan)
planner.getCascadesContext().getRewritePlan();
-
- logicalPlan
- .collect(plan -> plan instanceof LogicalProject
- && ((LogicalProject<?>) plan).child()
instanceof LogicalCatalogRelation)
- .stream().forEach(plan -> {
- LogicalProject logicalProject = (LogicalProject)
plan;
-
columns.addAll(logicalProject.getInputSlots().stream().map(Slot::getName)
- .collect(Collectors.toList()));
- });
+ Set<String> tempDisableRules =
connectContext.getSessionVariable().getDisableNereidsRuleNames();
+
connectContext.getSessionVariable().setDisableNereidsRules(SYNC_MV_PLANER_DISABLE_RULES);
+
connectContext.getStatementContext().invalidCache(SessionVariable.DISABLE_NEREIDS_RULES);
+ LogicalPlan logicalPlan;
+ try {
+ // disable rbo sync mv rewrite
+ connectContext.getSessionVariable()
+
.setVarOnce(SessionVariable.ENABLE_SYNC_MV_COST_BASED_REWRITE, "true");
+ // disable constant fold
+
connectContext.getSessionVariable().setVarOnce(SessionVariable.DEBUG_SKIP_FOLD_CONSTANT,
"true");
+ planner.planWithLock(unboundMvPlan, PhysicalProperties.ANY,
+ ExplainCommand.ExplainLevel.REWRITTEN_PLAN);
+ logicalPlan = (LogicalPlan)
planner.getCascadesContext().getRewritePlan();
+ } finally {
+ // after operate, roll back the disable rules
+
connectContext.getSessionVariable().setDisableNereidsRules(String.join(",",
tempDisableRules));
+
connectContext.getStatementContext().invalidCache(SessionVariable.DISABLE_NEREIDS_RULES);
+ }
+ Map<Boolean, List<Object>> partitionedPlan = logicalPlan
+ .collect(plan -> true)
+ .stream()
+ .collect(Collectors.partitioningBy(
+ plan -> plan instanceof LogicalProject
+ && ((LogicalProject<?>) plan).child()
instanceof LogicalCatalogRelation
+ ));
+ List<Object> projects = partitionedPlan.get(true);
+ if (projects.isEmpty()) {
+ // for scan
+ partitionedPlan.get(false)
+ .stream()
+ .filter(plan -> plan instanceof
LogicalCatalogRelation)
+ .map(plan -> (LogicalCatalogRelation) plan)
+ .forEach(plan ->
columns.addAll(logicalPlan.getOutput().stream().map(Slot::getName).collect(
+ Collectors.toList())));
+ } else {
+ // for projects
+ projects
+ .stream()
+ .map(plan -> (LogicalProject<?>) plan)
+ .forEach(plan ->
columns.addAll(plan.getInputSlots().stream().map(Slot::getName).collect(
+ Collectors.toList())));
+ }
} else {
throw new AnalysisException(String.format("can't parse %s ",
createMvSql));
}
diff --git
a/regression-test/data/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.out
b/regression-test/data/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.out
index b1feac9a13a..c8e17282628 100644
Binary files
a/regression-test/data/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.out
and
b/regression-test/data/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.out
differ
diff --git
a/regression-test/data/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.out
b/regression-test/data/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.out
index 3503eca2085..2c3de71bc3d 100644
Binary files
a/regression-test/data/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.out
and
b/regression-test/data/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.out
differ
diff --git
a/regression-test/suites/mv_p0/routine_load_mapping/routine_load_mapping.groovy
b/regression-test/suites/mv_p0/routine_load_mapping/routine_load_mapping.groovy
index ca3a3560e01..320224ad513 100644
---
a/regression-test/suites/mv_p0/routine_load_mapping/routine_load_mapping.groovy
+++
b/regression-test/suites/mv_p0/routine_load_mapping/routine_load_mapping.groovy
@@ -104,15 +104,8 @@ PROPERTIES (
heart_type = 1
;""")
- sql """ ALTER TABLE rt_new MODIFY COLUMN event_id VARCHAR(51) NULL;"""
- Thread.sleep(1000)
-
- streamLoad {
- table "rt_new"
- set 'column_separator', ','
- set 'columns',
'`battery_id`,`create_time`,`imei`,`event_id`,`event_name`,`heart_type`'
-
- file './test2'
- time 10000 // limit inflight 10s
+ test {
+ sql """ ALTER TABLE rt_new MODIFY COLUMN event_id VARCHAR(51) NULL;"""
+ exception "Can not modify column contained by mv"
}
}
diff --git
a/regression-test/suites/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.groovy
b/regression-test/suites/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.groovy
index dd5cc5c47ee..b4be34c76f6 100644
---
a/regression-test/suites/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.groovy
+++
b/regression-test/suites/schema_change_p0/modify_col_type_agg/schema_change_modify_mv_column_type_agg.groovy
@@ -73,25 +73,8 @@ suite("schema_change_modify_mv_column_type_agg") {
qt_sql "SELECT * from ${testTable} order by 1, 2, 3 limit 10"
qt_sql "SELECT * from ${testTable} where c_tinyint = 10 order by 1, 2, 3
limit 10 "
- sql """
- ALTER table ${testTable} MODIFY COLUMN c_int BIGINT max;
- """
- def getJobState = { tableName ->
- def jobStateResult = sql """ SHOW ALTER TABLE COLUMN WHERE
IndexName='${tableName}' ORDER BY createtime DESC LIMIT 1 """
- return jobStateResult[0][9]
- }
- int max_try_time = 100
- while (max_try_time--){
- String result = getJobState(testTable)
- if (result == "FINISHED") {
- break
- } else {
- sleep(2000)
- if (max_try_time < 1){
- assertEquals(1,2)
- }
- }
+ test {
+ sql """ ALTER table ${testTable} MODIFY COLUMN c_int BIGINT max """
+ exception "Can not modify column contained by mv"
}
- qt_master_sql """ desc ${testTable} all """
- sql "INSERT INTO ${testTable} SELECT * from ${testTable}"
}
diff --git
a/regression-test/suites/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.groovy
b/regression-test/suites/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.groovy
index 92c3870c563..f68bbea860e 100644
---
a/regression-test/suites/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.groovy
+++
b/regression-test/suites/schema_change_p0/modify_col_type_dup/schema_change_modify_mv_column_type.groovy
@@ -73,28 +73,8 @@ suite("schema_change_modify_mv_column_type") {
sql "set topn_opt_limit_threshold = 100"
qt_sql "SELECT * from ${testTable} order by 1, 2, 3 limit 10"
qt_sql "SELECT * from ${testTable} where c_tinyint = 10 order by 1, 2, 3
limit 10 "
-
- sql """
- ALTER table ${testTable} MODIFY COLUMN c_int BIGINT;
- """
- def getJobState = { tableName ->
- def jobStateResult = sql """ SHOW ALTER TABLE COLUMN WHERE
IndexName='${tableName}' ORDER BY createtime DESC LIMIT 1 """
- return jobStateResult[0][9]
- }
- int max_try_time = 100
- while (max_try_time--){
- String result = getJobState(testTable)
- if (result == "FINISHED") {
- break
- } else {
- sleep(2000)
- if (max_try_time < 1){
- assertEquals(1,2)
- }
- }
+ test {
+ sql "ALTER table ${testTable} MODIFY COLUMN c_int BIGINT;"
+ exception "Can not modify column contained by mv"
}
- // sync materialized view rewrite will fail when schema change, tmp
disable, enable when fixed
- sql """set enable_dml_materialized_view_rewrite = false;"""
- qt_master_sql """ desc ${testTable} all """
- sql "INSERT INTO ${testTable} SELECT * from ${testTable}"
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
