tool
/
starrocks
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[BugFix] Fix possible NPE in alter table (backport #62321) (#62340) 

Signed-off-by: shuming.li <ming.moriarty@gmail.com>
Co-authored-by: shuming.li <ming.moriarty@gmail.com>
This commit is contained in:
mergify[bot] 2025-08-26 21:18:52 +08:00 committed by GitHub
parent b69e6bd00f
commit e4e493480b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 155 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
fe/fe-core/src/main/java/com/starrocks/alter/AlterJobExecutor.java
View File

@ -610,7 +610,7 @@ public class AlterJobExecutor implements AstVisitor<Void, ConnectContext> {
Set<String> modifiedColumns = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
modifiedColumns.add(clause.getColName());
ErrorReport.wrapWithRuntimeException(() ->
schemaChangeHandler.checkModifiedColumWithMaterializedViews((OlapTable) table, modifiedColumns));
AlterMVJobExecutor.checkModifiedColumWithMaterializedViews((OlapTable) table, modifiedColumns));
GlobalStateMgr.getCurrentState().getLocalMetastore().renameColumn(db, table, clause);
// If modified columns are already done, inactive related mv

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

@ -19,12 +19,14 @@ import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.starrocks.analysis.Expr;
import com.starrocks.analysis.IntLiteral;
import com.starrocks.analysis.SlotRef;
import com.starrocks.analysis.StringLiteral;
import com.starrocks.analysis.TableName;
import com.starrocks.catalog.Column;
import com.starrocks.catalog.ColumnId;
import com.starrocks.catalog.Database;
import com.starrocks.catalog.KeysType;
import com.starrocks.catalog.MaterializedIndexMeta;
import com.starrocks.catalog.MaterializedView;
import com.starrocks.catalog.MvId;
import com.starrocks.catalog.MvPlanContext;
@ -78,6 +80,7 @@ import com.starrocks.warehouse.Warehouse;
import org.apache.commons.lang3.StringUtils;
import org.threeten.extra.PeriodDuration;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -696,4 +699,71 @@ public class AlterMVJobExecutor extends AlterJobExecutor {
}
}
}
/**
* Check related synchronous materialized views before modified columns, throw exceptions
* if modified columns affect the related rollup/synchronous mvs.
*/
public static void checkModifiedColumWithMaterializedViews(OlapTable olapTable,
Set<String> modifiedColumns) throws DdlException {
if (modifiedColumns == null || modifiedColumns.isEmpty()) {
return;
}
// If there is synchronized materialized view referring the column, throw exception.
if (olapTable.getIndexNameToId().size() > 1) {
Map<Long, MaterializedIndexMeta> metaMap = olapTable.getIndexIdToMeta();
for (Map.Entry<Long, MaterializedIndexMeta> entry : metaMap.entrySet()) {
Long id = entry.getKey();
if (id == olapTable.getBaseIndexId()) {
continue;
}
MaterializedIndexMeta meta = entry.getValue();
List<Column> schema = meta.getSchema();
String indexName = olapTable.getIndexNameById(id);
// ignore agg_keys type because it's like duplicated without agg functions
boolean hasAggregateFunction = olapTable.getKeysType() != KeysType.AGG_KEYS &&
schema.stream().anyMatch(x -> x.isAggregated());
if (hasAggregateFunction) {
for (Column rollupCol : schema) {
String colName = rollupCol.getName();
if (modifiedColumns.contains(colName)) {
throw new DdlException(String.format("Can not drop/modify the column %s, " +
"because the column is used in the related rollup %s, " +
"please drop the rollup index first.", colName, indexName));
}
if (rollupCol.getRefColumns() != null) {
for (SlotRef refColumn : rollupCol.getRefColumns()) {
String refColName = refColumn.getColumnName();
if (modifiedColumns.contains(refColName)) {
String defineExprSql = rollupCol.getDefineExpr() == null ? "" :
rollupCol.getDefineExpr().toSql();
throw new DdlException(String.format("Can not drop/modify the column %s, " +
"because the column is used in the related rollup %s " +
"with the define expr:%s, please drop the rollup index first.",
refColName, indexName, defineExprSql));
}
}
}
}
}
if (meta.getWhereClause() != null) {
Expr whereExpr = meta.getWhereClause();
List<SlotRef> whereSlots = new ArrayList<>();
whereExpr.collect(SlotRef.class, whereSlots);
for (SlotRef refColumn : whereSlots) {
String colName = refColumn.getColumnName();
if (modifiedColumns.contains(colName)) {
String whereExprSql = whereExpr.toSql();
throw new DdlException(String.format("Can not drop/modify the column %s, " +
"because the column is used in the related rollup %s " +
"with the where expr:%s, please drop the rollup index first.",
colName, indexName, whereExprSql));
}
}
}
}
}
}
}

71
fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeHandler.java
View File

@ -45,7 +45,6 @@ import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.starrocks.analysis.BloomFilterIndexUtil;
import com.starrocks.analysis.ColumnPosition;
import com.starrocks.analysis.Expr;
import com.starrocks.analysis.SlotRef;
import com.starrocks.binlog.BinlogConfig;
import com.starrocks.catalog.AggregateType;
@ -1932,7 +1931,7 @@ public class SchemaChangeHandler extends AlterHandler {
DropColumnClause dropColumnClause = (DropColumnClause) alterClause;
// check relative mvs with the modified column
Set<String> modifiedColumns = Set.of(dropColumnClause.getColName());
checkModifiedColumWithMaterializedViews(olapTable, modifiedColumns);
AlterMVJobExecutor.checkModifiedColumWithMaterializedViews(olapTable, modifiedColumns);
// drop column and drop indexes on this column
fastSchemaEvolution &=
@ -1943,7 +1942,7 @@ public class SchemaChangeHandler extends AlterHandler {
// check relative mvs with the modified column
Set<String> modifiedColumns = Set.of(modifyColumnClause.getColumn().getName());
checkModifiedColumWithMaterializedViews(olapTable, modifiedColumns);
AlterMVJobExecutor.checkModifiedColumWithMaterializedViews(olapTable, modifiedColumns);
// modify column
fastSchemaEvolution &= processModifyColumn(modifyColumnClause, olapTable, indexSchemaMap);
@ -1961,7 +1960,7 @@ public class SchemaChangeHandler extends AlterHandler {
}
AddFieldClause addFieldClause = (AddFieldClause) alterClause;
modifyFieldColumns = Set.of(addFieldClause.getColName());
checkModifiedColumWithMaterializedViews(olapTable, modifyFieldColumns);
AlterMVJobExecutor.checkModifiedColumWithMaterializedViews(olapTable, modifyFieldColumns);
int id = colUniqueIdSupplier.getAsInt();
processAddField((AddFieldClause) alterClause, olapTable, indexSchemaMap, id, newIndexes);
} else if (alterClause instanceof DropFieldClause) {
@ -1975,7 +1974,7 @@ public class SchemaChangeHandler extends AlterHandler {
}
DropFieldClause dropFieldClause = (DropFieldClause) alterClause;
modifyFieldColumns = Set.of(dropFieldClause.getColName());
checkModifiedColumWithMaterializedViews(olapTable, modifyFieldColumns);
AlterMVJobExecutor.checkModifiedColumWithMaterializedViews(olapTable, modifyFieldColumns);
processDropField((DropFieldClause) alterClause, olapTable, indexSchemaMap, newIndexes);
} else if (alterClause instanceof ReorderColumnsClause) {
// reorder column
@ -2033,68 +2032,6 @@ public class SchemaChangeHandler extends AlterHandler {
}
}
/**
* Check related synchronous materialized views before modified columns, throw exceptions
* if modified columns affect the related rollup/synchronous mvs.
*/
public void checkModifiedColumWithMaterializedViews(OlapTable olapTable,
Set<String> modifiedColumns) throws DdlException {
if (modifiedColumns == null || modifiedColumns.isEmpty()) {
return;
}
// If there is synchronized materialized view referring the column, throw exception.
if (olapTable.getIndexNameToId().size() > 1) {
Map<Long, MaterializedIndexMeta> metaMap = olapTable.getIndexIdToMeta();
for (Map.Entry<Long, MaterializedIndexMeta> entry : metaMap.entrySet()) {
Long id = entry.getKey();
if (id == olapTable.getBaseIndexId()) {
continue;
}
MaterializedIndexMeta meta = entry.getValue();
List<Column> schema = meta.getSchema();
// ignore agg_keys type because it's like duplicated without agg functions
boolean hasAggregateFunction = olapTable.getKeysType() != KeysType.AGG_KEYS &&
schema.stream().anyMatch(x -> x.isAggregated());
if (hasAggregateFunction) {
for (Column rollupCol : schema) {
if (modifiedColumns.contains(rollupCol.getName())) {
throw new DdlException(String.format("Can not drop/modify the column %s, " +
"because the column is used in the related rollup %s, " +
"please drop the rollup index first.",
rollupCol.getName(), olapTable.getIndexNameById(meta.getIndexId())));
}
if (rollupCol.getRefColumns() != null) {
for (SlotRef refColumn : rollupCol.getRefColumns()) {
if (modifiedColumns.contains(refColumn.getColumnName())) {
throw new DdlException(String.format("Can not drop/modify the column %s, " +
"because the column is used in the related rollup %s " +
"with the define expr:%s, please drop the rollup index first.",
rollupCol.getName(), olapTable.getIndexNameById(meta.getIndexId()),
rollupCol.getDefineExpr().toSql()));
}
}
}
}
}
if (meta.getWhereClause() != null) {
Expr whereExpr = meta.getWhereClause();
List<SlotRef> whereSlots = new ArrayList<>();
whereExpr.collect(SlotRef.class, whereSlots);
for (SlotRef refColumn : whereSlots) {
if (modifiedColumns.contains(refColumn.getColumnName())) {
throw new DdlException(String.format("Can not drop/modify the column %s, " +
"because the column is used in the related rollup %s " +
"with the where expr:%s, please drop the rollup index first.",
refColumn.getColumn().getName(), olapTable.getIndexNameById(meta.getIndexId()),
meta.getWhereClause().toSql()));
}
}
}
}
}
}
@Override
public ShowResultSet process(List<AlterClause> alterClauses, Database db, OlapTable olapTable)
throws StarRocksException {

10
fe/fe-core/src/test/java/com/starrocks/alter/SchemaChangeHandlerWithMVTest.java
View File

@ -222,9 +222,8 @@ public class SchemaChangeHandlerWithMVTest extends TestWithFeService {
"alter table sc_dup3 drop column error_code");
} catch (Exception e) {
Assertions.assertTrue(
e.getMessage().contains("Can not drop/modify the column mv_count_error_code, because the column " +
"is used in the related rollup mv1 with the define expr:" +
"CASE WHEN `test`.`sc_dup3`.`error_code` IS NULL THEN 0 ELSE 1 END, please drop the rollup index first."));
e.getMessage().contains("Can not drop/modify the column error_code, " +
"because the column is used in the related rollup mv1 with the define exp"), e.getMessage());
}
}
@ -237,9 +236,8 @@ public class SchemaChangeHandlerWithMVTest extends TestWithFeService {
"group by timestamp",
"alter table sc_dup3 modify column error_code BIGINT");
} catch (Exception e) {
Assertions.assertTrue(e.getMessage().contains("Can not drop/modify the column mv_count_error_code, because " +
"the column is used in the related rollup mv1 with the define expr:" +
"CASE WHEN `test`.`sc_dup3`.`error_code` IS NULL THEN 0 ELSE 1 END, please drop the rollup index first."));
Assertions.assertTrue(e.getMessage().contains("Can not drop/modify the column error_code, because " +
"the column is used in the related rollup mv1 with the define expr"), e.getMessage());
}
}

51
test/sql/test_alter_table/R/test_alter_table_with_mv Normal file
View File

@ -0,0 +1,51 @@
-- name: test_alter_table_with_mv
CREATE TABLE t0(k0 BIGINT, k1 DATETIME, v0 INT, v1 VARCHAR(100))
distributed by hash(k0)
order by (v0)
properties('replication_num'='1');
-- result:
-- !result
INSERT INTO t0 VALUES(0, '2024-01-01 00:00:00', 10, '100');
-- result:
-- !result
CREATE MATERIALIZED VIEW test_mv1 AS SELECT k0, v1 FROM t0 WHERE v0 > 10;
-- result:
-- !result
function: wait_table_rollup_finish()
-- result:
None
-- !result
CREATE MATERIALIZED VIEW test_mv2 AS SELECT k0, v1, k1, v0 FROM t0 WHERE v0 > 10 and k1 = '2024-01-01 00:00:00';
-- result:
-- !result
function: wait_table_rollup_finish()
-- result:
None
-- !result
ALTER TABLE t0 DROP COLUMN k1;
-- result:
[REGEX].*Can not drop/modify the column k1, because the column is used in the related rollup.*
-- !result
ALTER TABLE t0 DROP COLUMN v0;
-- result:
[REGEX].*Can not drop/modify the column v0, because the column is used in the related rollup.*
-- !result
ALTER TABLE t0 DROP COLUMN v1, DROP COLUMN v0;
-- result:
[REGEX].*Can not drop/modify the column v0, because the column is used in the related rollup.*
-- !result
ALTER TABLE t0 DROP COLUMN v1, DROP COLUMN v100;
-- result:
E: (1064, 'Column does not exists: v100')
-- !result
DESC t0;
-- result:
k0 bigint YES true None
k1 datetime YES true None
v0 int YES true None
v1 varchar(100) YES false None
-- !result
SELECT * from t0 order by k0;
-- result:
0 2024-01-01 00:00:00 10 100
-- !result

25
test/sql/test_alter_table/T/test_alter_table_with_mv Normal file
View File

@ -0,0 +1,25 @@
-- name: test_alter_table_with_mv
CREATE TABLE t0(k0 BIGINT, k1 DATETIME, v0 INT, v1 VARCHAR(100))
distributed by hash(k0)
order by (v0)
properties('replication_num'='1');
INSERT INTO t0 VALUES(0, '2024-01-01 00:00:00', 10, '100');
CREATE MATERIALIZED VIEW test_mv1 AS SELECT k0, v1 FROM t0 WHERE v0 > 10;
function: wait_table_rollup_finish()
CREATE MATERIALIZED VIEW test_mv2 AS SELECT k0, v1, k1, v0 FROM t0 WHERE v0 > 10 and k1 = '2024-01-01 00:00:00';
function: wait_table_rollup_finish()
ALTER TABLE t0 DROP COLUMN k1;
ALTER TABLE t0 DROP COLUMN v0;
ALTER TABLE t0 DROP COLUMN v1, DROP COLUMN v0;
ALTER TABLE t0 DROP COLUMN v1, DROP COLUMN v100;
DESC t0;
SELECT * from t0 order by k0;