You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2022/12/14 05:58:58 UTC
[doris] 02/12: [fix](schemachange) fix the schema change that causes the be core dump. (#14804)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-1.2-lts
in repository https://gitbox.apache.org/repos/asf/doris.git
commit 9768a4d52ed829f7b43ef68c2d47bdf4f58f96e1
Author: luozenglin <37...@users.noreply.github.com>
AuthorDate: Thu Dec 8 17:36:54 2022 +0800
[fix](schemachange) fix the schema change that causes the be core dump. (#14804)
* [fix](schemachange) fix the schema change that causes the be core dump.
Forbid schema change to add or modify the key column of the agg model as double or float.
---
.../apache/doris/alter/SchemaChangeHandler.java | 5 ++---
.../org/apache/doris/analysis/AddColumnClause.java | 15 ++++++++++++++-
.../apache/doris/analysis/AlterTableClause.java | 6 ++++++
.../org/apache/doris/analysis/AlterTableStmt.java | 3 +++
.../apache/doris/analysis/ModifyColumnClause.java | 15 ++++++++++++++-
.../apache/doris/alter/SchemaChangeJobV2Test.java | 5 +++--
.../apache/doris/analysis/AddColumnClauseTest.java | 11 ++++++-----
.../doris/analysis/ModifyColumnClauseTest.java | 5 +++--
.../test_agg_keys_schema_change.groovy | 22 ++++++++++++++++++++++
9 files changed, 73 insertions(+), 14 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 39395d0039..a14b03aa31 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
@@ -492,9 +492,8 @@ public class SchemaChangeHandler extends AlterHandler {
if (KeysType.AGG_KEYS == olapTable.getKeysType()) {
if (modColumn.isKey() && null != modColumn.getAggregationType()) {
throw new DdlException("Can not assign aggregation method on key column: " + modColumn.getName());
- } else if (null == modColumn.getAggregationType()) {
- // in aggregate key table, no aggregation method indicate key column
- modColumn.setIsKey(true);
+ } else if (!modColumn.isKey() && null == modColumn.getAggregationType()) {
+ throw new DdlException("Aggregate method must be specified for value column: " + modColumn.getName());
}
} else if (KeysType.UNIQUE_KEYS == olapTable.getKeysType()) {
if (null != modColumn.getAggregationType()) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AddColumnClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AddColumnClause.java
index 03afe36ed5..2d472cd549 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AddColumnClause.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AddColumnClause.java
@@ -19,7 +19,12 @@ package org.apache.doris.analysis;
import org.apache.doris.alter.AlterOpType;
import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.KeysType;
+import org.apache.doris.catalog.OlapTable;
+import org.apache.doris.catalog.Table;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
@@ -64,10 +69,18 @@ public class AddColumnClause extends AlterTableClause {
}
@Override
- public void analyze(Analyzer analyzer) throws AnalysisException {
+ public void analyze(Analyzer analyzer) throws AnalysisException, DdlException {
if (columnDef == null) {
throw new AnalysisException("No column definition in add column clause.");
}
+ if (tableName != null) {
+ Table table = Env.getCurrentInternalCatalog().getDbOrDdlException(tableName.getDb())
+ .getTableOrDdlException(tableName.getTbl());
+ if (table instanceof OlapTable && ((OlapTable) table).getKeysType() == KeysType.AGG_KEYS
+ && columnDef.getAggregateType() == null) {
+ columnDef.setIsKey(true);
+ }
+ }
columnDef.analyze(true);
if (colPos != null) {
colPos.analyze();
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableClause.java
index 7a223beb6f..ad611bb95a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableClause.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableClause.java
@@ -29,7 +29,13 @@ public abstract class AlterTableClause extends AlterClause {
// if set to true, the corresponding table should be stable before processing this operation on it.
protected boolean needTableStable = true;
+ protected TableName tableName;
+
public boolean isNeedTableStable() {
return needTableStable;
}
+
+ public void setTableName(TableName tableName) {
+ this.tableName = tableName;
+ }
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableStmt.java
index 4435b7e608..4c5bf46e9b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableStmt.java
@@ -78,6 +78,9 @@ public class AlterTableStmt extends DdlStmt {
ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_ALTER_OPERATION);
}
for (AlterClause op : ops) {
+ if (op instanceof AlterTableClause) {
+ ((AlterTableClause) op).setTableName(tbl);
+ }
op.analyze(analyzer);
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyColumnClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyColumnClause.java
index afa369f089..5477614006 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyColumnClause.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyColumnClause.java
@@ -19,7 +19,12 @@ package org.apache.doris.analysis;
import org.apache.doris.alter.AlterOpType;
import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.KeysType;
+import org.apache.doris.catalog.OlapTable;
+import org.apache.doris.catalog.Table;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
import com.google.common.base.Strings;
@@ -59,10 +64,18 @@ public class ModifyColumnClause extends AlterTableClause {
}
@Override
- public void analyze(Analyzer analyzer) throws AnalysisException {
+ public void analyze(Analyzer analyzer) throws AnalysisException, DdlException {
if (columnDef == null) {
throw new AnalysisException("No column definition in modify column clause.");
}
+ if (tableName != null) {
+ Table table = Env.getCurrentInternalCatalog().getDbOrDdlException(tableName.getDb())
+ .getTableOrDdlException(tableName.getTbl());
+ if (table instanceof OlapTable && ((OlapTable) table).getKeysType() == KeysType.AGG_KEYS
+ && columnDef.getAggregateType() == null) {
+ columnDef.setIsKey(true);
+ }
+ }
columnDef.analyze(true);
if (colPos != null) {
colPos.analyze();
diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
index 323547b494..954bc63706 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
@@ -108,8 +108,9 @@ public class SchemaChangeJobV2Test {
public ExpectedException expectedEx = ExpectedException.none();
@Before
- public void setUp() throws InstantiationException, IllegalAccessException, IllegalArgumentException,
- InvocationTargetException, NoSuchMethodException, SecurityException, AnalysisException {
+ public void setUp()
+ throws InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException,
+ NoSuchMethodException, SecurityException, AnalysisException, DdlException {
FakeEnv.setMetaVersion(FeMetaVersion.VERSION_CURRENT);
fakeEditLog = new FakeEditLog();
fakeEnv = new FakeEnv();
diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/AddColumnClauseTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/AddColumnClauseTest.java
index 8fabe62850..7a97d702fd 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/AddColumnClauseTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/AddColumnClauseTest.java
@@ -22,6 +22,7 @@ import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.ScalarType;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
import mockit.Expectations;
import mockit.Mocked;
@@ -41,7 +42,7 @@ public class AddColumnClauseTest {
}
@Test
- public void testNormal() throws AnalysisException {
+ public void testNormal() throws AnalysisException, DdlException {
Column column = new Column("testCol", ScalarType.createType(PrimitiveType.INT));
new Expectations() {
{
@@ -91,14 +92,14 @@ public class AddColumnClauseTest {
}
@Test(expected = AnalysisException.class)
- public void testNoColDef() throws AnalysisException {
+ public void testNoColDef() throws AnalysisException, DdlException {
AddColumnClause clause = new AddColumnClause(null, null, null, null);
clause.analyze(analyzer);
Assert.fail("No exception throws.");
}
@Test(expected = AnalysisException.class)
- public void testNoDefault() throws AnalysisException {
+ public void testNoDefault() throws AnalysisException, DdlException {
new Expectations() {
{
definition.analyze(true);
@@ -131,7 +132,7 @@ public class AddColumnClauseTest {
}
@Test(expected = AnalysisException.class)
- public void testAggPos() throws AnalysisException {
+ public void testAggPos() throws AnalysisException, DdlException {
new Expectations() {
{
definition.analyze(true);
@@ -164,7 +165,7 @@ public class AddColumnClauseTest {
}
@Test(expected = AnalysisException.class)
- public void testAddValueToFirst() throws AnalysisException {
+ public void testAddValueToFirst() throws AnalysisException, DdlException {
new Expectations() {
{
definition.analyze(true);
diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/ModifyColumnClauseTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/ModifyColumnClauseTest.java
index cf184d8e80..93119403c3 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/analysis/ModifyColumnClauseTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/ModifyColumnClauseTest.java
@@ -20,6 +20,7 @@ package org.apache.doris.analysis;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
import mockit.Expectations;
import mockit.Mocked;
@@ -36,7 +37,7 @@ public class ModifyColumnClauseTest {
}
@Test
- public void testNormal(@Mocked ColumnDef definition) throws AnalysisException {
+ public void testNormal(@Mocked ColumnDef definition) throws AnalysisException, DdlException {
Column column = new Column("tsetCol", PrimitiveType.INT);
new Expectations() {
{
@@ -76,7 +77,7 @@ public class ModifyColumnClauseTest {
}
@Test(expected = AnalysisException.class)
- public void testNoColDef() throws AnalysisException {
+ public void testNoColDef() throws AnalysisException, DdlException {
ModifyColumnClause clause = new ModifyColumnClause(null, null, null, null);
clause.analyze(analyzer);
Assert.fail("No exception throws.");
diff --git a/regression-test/suites/schema_change_p0/test_agg_keys_schema_change.groovy b/regression-test/suites/schema_change_p0/test_agg_keys_schema_change.groovy
index 20691b3f63..5cf7fd59b5 100644
--- a/regression-test/suites/schema_change_p0/test_agg_keys_schema_change.groovy
+++ b/regression-test/suites/schema_change_p0/test_agg_keys_schema_change.groovy
@@ -122,6 +122,28 @@ suite ("test_agg_keys_schema_change") {
qt_sc """ select count(*) from ${tableName} """
+ // test add double or float key column
+ test {
+ sql "ALTER table ${tableName} ADD COLUMN new_key_column_double DOUBLE"
+ exception "Float or double can not used as a key, use decimal instead."
+ }
+
+ test {
+ sql "ALTER table ${tableName} ADD COLUMN new_key_column_float FLOAT"
+ exception "Float or double can not used as a key, use decimal instead."
+ }
+
+ // test modify key column type to double or float
+ test {
+ sql "ALTER table ${tableName} MODIFY COLUMN age FLOAT"
+ exception "Float or double can not used as a key, use decimal instead."
+ }
+
+ test {
+ sql "ALTER table ${tableName} MODIFY COLUMN age DOUBLE"
+ exception "Float or double can not used as a key, use decimal instead."
+ }
+
// drop key column, not light schema change
sql """
ALTER TABLE ${tableName} DROP COLUMN new_key_column
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org