database/doris
2
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Fix bug that user can set null default value to non-nullable column in create table stmt (#1453)

Browse Source 
In create table stmt, column definition `k1 INT NOT NULL DEFAULT NULL`
should not be allowed
This commit is contained in:
Mingyu Chen
2019-07-10 23:48:29 +08:00
committed by ZHAO Chun
parent 98bd4b4565
commit 9c96a688c3
5 changed files with 69 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
fe/src/main/cup/sql_parser.cup
View File

@ -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;
:}
;

61
fe/src/main/java/org/apache/doris/analysis/ColumnDef.java
View File

@ -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

3
fe/src/test/java/org/apache/doris/alter/SchemaChangeJobTest.java
View File

@ -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

9
fe/src/test/java/org/apache/doris/analysis/AddColumnsClauseTest.java
View File

@ -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<ColumnDef> 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);

17
fe/src/test/java/org/apache/doris/analysis/ColumnDefTest.java
View File

@ -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);
}