Skip to content

[fix](schemachange) fix the schema change that causes the be core dump. #14804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yangzhg merged 5 commits into from
Dec 8, 2022
Merged

[fix](schemachange) fix the schema change that causes the be core dump. #14804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ace42f2
[fix](schemachange) fix the schema change that causes the be core dump.
luozenglin Dec 4, 2022
51b9c76
update
luozenglin Dec 6, 2022
7bd07bb
update
luozenglin Dec 7, 2022
26193bb
update
luozenglin Dec 7, 2022
903f0e7
update
luozenglin Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,9 +492,8 @@ private void processModifyColumn(ModifyColumnClause alterClause, OlapTable olapT
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()) {
Expand Down
15 changes: 14 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/analysis/AddColumnClause.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@

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;

Expand Down Expand Up @@ -64,10 +69,18 @@ public AddColumnClause(ColumnDef columnDef, ColumnPosition colPos, String rollup
}

@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();
Expand Down
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableClause.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ public AlterTableClause(AlterOpType opType) {
// 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;
}
}
3 changes: 3 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableStmt.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ public void analyze(Analyzer analyzer) throws UserException {
ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_ALTER_OPERATION);
}
for (AlterClause op : ops) {
if (op instanceof AlterTableClause) {
((AlterTableClause) op).setTableName(tbl);
}
op.analyze(analyzer);
}
}
Expand Down
15 changes: 14 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyColumnClause.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@

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;

Expand Down Expand Up @@ -59,10 +64,18 @@ public ModifyColumnClause(ColumnDef columnDef, ColumnPosition colPos, String rol
}

@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();
Expand Down
5 changes: 3 additions & 2 deletions fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeJobV2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
11 changes: 6 additions & 5 deletions fe/fe-core/src/test/java/org/apache/doris/analysis/AddColumnClauseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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;
Expand All @@ -41,7 +42,7 @@ public static void setUp() {
}

@Test
public void testNormal() throws AnalysisException {
public void testNormal() throws AnalysisException, DdlException {
Column column = new Column("testCol", ScalarType.createType(PrimitiveType.INT));
new Expectations() {
{
Expand Down Expand Up @@ -91,14 +92,14 @@ public void testNormal() throws AnalysisException {
}

@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);
Expand Down Expand Up @@ -131,7 +132,7 @@ public void testNoDefault() throws AnalysisException {
}

@Test(expected = AnalysisException.class)
public void testAggPos() throws AnalysisException {
public void testAggPos() throws AnalysisException, DdlException {
new Expectations() {
{
definition.analyze(true);
Expand Down Expand Up @@ -164,7 +165,7 @@ public void testAggPos() throws AnalysisException {
}

@Test(expected = AnalysisException.class)
public void testAddValueToFirst() throws AnalysisException {
public void testAddValueToFirst() throws AnalysisException, DdlException {
new Expectations() {
{
definition.analyze(true);
Expand Down
5 changes: 3 additions & 2 deletions fe/fe-core/src/test/java/org/apache/doris/analysis/ModifyColumnClauseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
Expand All @@ -36,7 +37,7 @@ public static void setUp() {
}

@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() {
{
Expand Down Expand Up @@ -76,7 +77,7 @@ public void testNormal(@Mocked ColumnDef definition) throws AnalysisException {
}

@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.");
Expand Down
22 changes: 22 additions & 0 deletions regression-test/suites/schema_change_p0/test_agg_keys_schema_change.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down