diff --git a/fe/src/main/cup/sql_parser.cup b/fe/src/main/cup/sql_parser.cup index 001e591b0a..c170404312 100644 --- a/fe/src/main/cup/sql_parser.cup +++ b/fe/src/main/cup/sql_parser.cup @@ -405,7 +405,8 @@ nonterminal String keyword, ident, ident_or_text, variable_name, text_or_passwor charset_name_or_default, old_or_new_charset_name_or_default, opt_collate, collation_name_or_default, type_func_name_keyword, type_function_name, opt_file_format; -nonterminal String opt_db, opt_partition_name, procedure_or_function, opt_default_value, opt_comment, opt_engine; +nonterminal String opt_db, opt_partition_name, procedure_or_function, opt_comment, opt_engine; +nonterminal ColumnDef.DefaultValue opt_default_value; nonterminal Boolean opt_if_exists, opt_if_not_exists; nonterminal Boolean opt_external; @@ -1571,15 +1572,15 @@ column_definition_list ::= opt_default_value ::= /* Empty */ {: - RESULT = null; + RESULT = ColumnDef.DefaultValue.NOT_SET; :} | KW_DEFAULT STRING_LITERAL:value {: - RESULT = value; + RESULT = new ColumnDef.DefaultValue(true, value); :} | KW_DEFAULT KW_NULL {: - RESULT = null; + RESULT = ColumnDef.DefaultValue.NULL_DEFAULT_VALUE; :} ; diff --git a/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java b/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java index 1ff3b09ba5..c69cd3646f 100644 --- a/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java +++ b/fe/src/main/java/org/apache/doris/analysis/ColumnDef.java @@ -38,7 +38,39 @@ import org.apache.logging.log4j.Logger; // pv bigint sum NULL DEFAULT "-1" "page visit" public class ColumnDef { private static final Logger LOG = LogManager.getLogger(ColumnDef.class); - private static final String HLL_EMPTY_SET = "0"; + + /* + * User can set default value for a column + * eg: + * k1 INT NOT NULL DEFAULT "10" + * k1 INT NULL + * k1 INT NULL DEFAULT NULL + * + * ColumnnDef will be transformed to Column in Analysis phase, and in Column, default value is a String. + * No matter does the user set the default value as NULL explicitly, or not set default value, + * the default value in Column will be "null", so that Doris can not distinguish between "not set" and "set as null". + * + * But this is OK because Column has another attribute "isAllowNull". + * If the column is not allowed to be null, and user does not set the default value, + * even if default value saved in Column is null, the "null" value can not be loaded into this column, + * so data correctness can be guaranteed. + */ + public static class DefaultValue { + public boolean isSet; + public String value; + + public DefaultValue(boolean isSet, String value) { + this.isSet = isSet; + this.value = value; + } + + // no default value + public static DefaultValue NOT_SET = new DefaultValue(false, null); + // default null + public static DefaultValue NULL_DEFAULT_VALUE = new DefaultValue(true, null); + // default "value" + public static DefaultValue HLL_EMPTY_DEFAULT_VALUE = new DefaultValue(true, "0"); + } // parameter initialized in constructor private String name; @@ -46,19 +78,18 @@ public class ColumnDef { private AggregateType aggregateType; private boolean isKey; private boolean isAllowNull; - private String defaultValue; + private DefaultValue defaultValue; private String comment; public ColumnDef(String name, TypeDef typeDef) { this.name = name; this.typeDef = typeDef; this.comment = ""; + this.defaultValue = DefaultValue.NOT_SET; } - public ColumnDef(String name, TypeDef typeDef, - boolean isKey, AggregateType aggregateType, - boolean isAllowNull, String defaultValue, - String comment) { + public ColumnDef(String name, TypeDef typeDef, boolean isKey, AggregateType aggregateType, + boolean isAllowNull, DefaultValue defaultValue, String comment) { this.name = name; this.typeDef = typeDef; this.isKey = isKey; @@ -69,7 +100,7 @@ public class ColumnDef { } public boolean isAllowNull() { return isAllowNull; } - public String getDefaultValue() { return defaultValue; } + public String getDefaultValue() { return defaultValue.value; } public String getName() { return name; } public AggregateType getAggregateType() { return aggregateType; } public void setAggregateType(AggregateType aggregateType, boolean xxx) { this.aggregateType = aggregateType; } @@ -122,11 +153,15 @@ public class ColumnDef { if (defaultValue != null) { throw new AnalysisException("Hll can not set default value"); } - defaultValue = HLL_EMPTY_SET; + defaultValue = DefaultValue.HLL_EMPTY_DEFAULT_VALUE; } - if (defaultValue != null) { - validateDefaultValue(type, defaultValue); + if (!isAllowNull && defaultValue == DefaultValue.NULL_DEFAULT_VALUE) { + throw new AnalysisException("Can not set null default value to non nullable column: " + name); + } + + if (defaultValue.isSet && defaultValue.value != null) { + validateDefaultValue(type, defaultValue.value); } } @@ -191,8 +226,8 @@ public class ColumnDef { sb.append("NOT NULL "); } - if (defaultValue != null) { - sb.append("DEFAULT \"").append(defaultValue).append("\" "); + if (defaultValue.isSet) { + sb.append("DEFAULT \"").append(defaultValue.value).append("\" "); } sb.append("COMMENT \"").append(comment).append("\""); @@ -200,7 +235,7 @@ public class ColumnDef { } public Column toColumn() { - return new Column(name, typeDef.getType(), isKey, aggregateType, isAllowNull, defaultValue, comment); + return new Column(name, typeDef.getType(), isKey, aggregateType, isAllowNull, defaultValue.value, comment); } @Override diff --git a/fe/src/test/java/org/apache/doris/alter/SchemaChangeJobTest.java b/fe/src/test/java/org/apache/doris/alter/SchemaChangeJobTest.java index 2a25424a8d..f37704569b 100644 --- a/fe/src/test/java/org/apache/doris/alter/SchemaChangeJobTest.java +++ b/fe/src/test/java/org/apache/doris/alter/SchemaChangeJobTest.java @@ -25,6 +25,7 @@ import org.apache.doris.analysis.AddColumnClause; import org.apache.doris.analysis.AlterClause; import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.ColumnDef; +import org.apache.doris.analysis.ColumnDef.DefaultValue; import org.apache.doris.analysis.ColumnPosition; import org.apache.doris.analysis.TypeDef; import org.apache.doris.catalog.AggregateType; @@ -84,7 +85,7 @@ public class SchemaChangeJobTest { private String transactionSource = "localfe"; private static Analyzer analyzer; private static ColumnDef newCol = new ColumnDef("add_v", new TypeDef(ScalarType.createType(PrimitiveType.INT)), false, AggregateType.MAX, - false, "1", ""); + false, new DefaultValue(true, "1"), ""); private static AddColumnClause addColumnClause = new AddColumnClause(newCol, new ColumnPosition("v"), null, null); @Before diff --git a/fe/src/test/java/org/apache/doris/analysis/AddColumnsClauseTest.java b/fe/src/test/java/org/apache/doris/analysis/AddColumnsClauseTest.java index 88c305cd69..da1c1d5244 100644 --- a/fe/src/test/java/org/apache/doris/analysis/AddColumnsClauseTest.java +++ b/fe/src/test/java/org/apache/doris/analysis/AddColumnsClauseTest.java @@ -17,9 +17,9 @@ package org.apache.doris.analysis; -import org.apache.doris.catalog.Column; -import org.apache.doris.catalog.ScalarType; +import org.apache.doris.analysis.ColumnDef.DefaultValue; import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.ScalarType; import org.apache.doris.common.AnalysisException; import com.google.common.collect.Lists; @@ -42,9 +42,10 @@ public class AddColumnsClauseTest { public void testNormal() throws AnalysisException { List columns = Lists.newArrayList(); ColumnDef definition = new ColumnDef("col1", new TypeDef(ScalarType.createType(PrimitiveType.INT)), - true, null, false,"0", ""); + true, null, false, new DefaultValue(true, "0"), ""); columns.add(definition); - definition = new ColumnDef("col2", new TypeDef(ScalarType.createType(PrimitiveType.INT)), true, null, false, "0", ""); + definition = new ColumnDef("col2", new TypeDef(ScalarType.createType(PrimitiveType.INT)), true, null, false, + new DefaultValue(true, "0"), ""); columns.add(definition); AddColumnsClause clause = new AddColumnsClause(columns, null, null); clause.analyze(analyzer); diff --git a/fe/src/test/java/org/apache/doris/analysis/ColumnDefTest.java b/fe/src/test/java/org/apache/doris/analysis/ColumnDefTest.java index 0c71473dfc..48f17ef5a8 100644 --- a/fe/src/test/java/org/apache/doris/analysis/ColumnDefTest.java +++ b/fe/src/test/java/org/apache/doris/analysis/ColumnDefTest.java @@ -17,15 +17,16 @@ package org.apache.doris.analysis; +import org.apache.doris.analysis.ColumnDef.DefaultValue; +import org.apache.doris.catalog.AggregateType; +import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.ScalarType; +import org.apache.doris.common.AnalysisException; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.apache.doris.catalog.AggregateType; -import org.apache.doris.catalog.ScalarType; -import org.apache.doris.catalog.PrimitiveType; -import org.apache.doris.common.AnalysisException; - public class ColumnDefTest { private TypeDef intCol; private TypeDef stringCol; @@ -50,14 +51,14 @@ public class ColumnDefTest { Assert.assertNull(column.getDefaultValue()); // default - column = new ColumnDef("col", intCol, true, null, false, "10", ""); + column = new ColumnDef("col", intCol, true, null, false, new DefaultValue(true, "10"), ""); column.analyze(true); Assert.assertNull(column.getAggregateType()); Assert.assertEquals("10", column.getDefaultValue()); Assert.assertEquals("`col` int(11) NOT NULL DEFAULT \"10\" COMMENT \"\"", column.toSql()); // agg - column = new ColumnDef("col", floatCol, false, AggregateType.SUM, false, "10", ""); + column = new ColumnDef("col", floatCol, false, AggregateType.SUM, false, new DefaultValue(true, "10"), ""); column.analyze(true); Assert.assertEquals("10", column.getDefaultValue()); Assert.assertEquals(AggregateType.SUM, column.getAggregateType()); @@ -73,7 +74,7 @@ public class ColumnDefTest { @Test(expected = AnalysisException.class) public void testStrSum() throws AnalysisException { - ColumnDef column = new ColumnDef("col", stringCol, false, AggregateType.SUM, true, null, ""); + ColumnDef column = new ColumnDef("col", stringCol, false, AggregateType.SUM, true, DefaultValue.NOT_SET, ""); column.analyze(true); }