Skip to content

Commit

Permalink
[Enhancement](partition) Forbid create table with null partition item…
Browse files Browse the repository at this point in the history 
… which relative column is not null (apache#39449)

Issue Number: close #xxx

before:
```sql
CREATE TABLE `test_null` (
`k0` BIGINT NOT NULL,
`k1` BIGINT NOT NULL
)
partition by list (k0, k1) (
PARTITION `pX` values in ((NULL, 1))
)
PROPERTIES (
"replication_allocation" = "tag.location.default: 1"
);
```
may core in local exchange for inserting.

now:
```sql
mysql [test]>CREATE TABLE `test_null` (
    -> `k0` BIGINT NOT NULL,
    -> `k1` BIGINT NOT NULL
    -> )
    -> partition by list (k0, k1) (
    -> PARTITION `pX` values in ((NULL, 1))
    -> )
    -> PROPERTIES (
    -> "replication_allocation" = "tag.location.default: 1"
    -> );
ERROR 1105 (HY000): errCode = 2, detailMessage = errCode = 2, detailMessage = Can't have null partition is for NOT NULL partition column in partition expr's index 0
```
  • Loading branch information
@zclllyybb
zclllyybb committed Aug 23, 2024
1 parent 1177d27 commit e0af14c
Show file tree
Hide file tree
Showing 6 changed files with 225 additions and 73 deletions.
128 changes: 100 additions & 28 deletions fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import org.apache.doris.analysis.LiteralExpr;
import org.apache.doris.analysis.PartitionDesc;
import org.apache.doris.analysis.PartitionKeyDesc;
import org.apache.doris.analysis.PartitionKeyDesc.PartitionKeyValueType;
import org.apache.doris.analysis.PartitionValue;
import org.apache.doris.analysis.QueryStmt;
import org.apache.doris.analysis.RecoverDbStmt;
import org.apache.doris.analysis.RecoverPartitionStmt;
Expand Down Expand Up @@ -2116,6 +2118,98 @@ private Type getChildTypeByName(String name, CreateTableStmt stmt)
throw new AnalysisException("Cannot find column `" + name + "` in table's columns");
}

private boolean findAllowNullforSlotRef(List<Column> baseSchema, SlotRef slot) throws AnalysisException {
for (Column col : baseSchema) {
if (col.nameEquals(slot.getColumnName(), true)) {
return col.isAllowNull();
}
}
throw new AnalysisException("Unknown partition column name:" + slot.getColumnName());
}

private void checkNullityEqual(ArrayList<Boolean> partitionSlotNullables, List<PartitionValue> item)
throws AnalysisException {
// for MAX_VALUE or somethings
if (item == null) {
return;
}
for (int i = 0; i < item.size(); i++) {
try {
if (!partitionSlotNullables.get(i) && item.get(i).isNullPartition()) {
throw new AnalysisException("Can't have null partition is for NOT NULL partition "
+ "column in partition expr's index " + i);
}
} catch (IndexOutOfBoundsException e) {
throw new AnalysisException("partition item's size out of partition columns: " + e.getMessage());
}
}
}

private void checkPartitionNullity(List<Column> baseSchema, PartitionDesc partitionDesc,
SinglePartitionDesc partition)
throws AnalysisException {
// in creating OlapTable, expr.desc is null. so we should find the column ourself.
ArrayList<Expr> partitionExprs = partitionDesc.getPartitionExprs();
ArrayList<Boolean> partitionSlotNullables = new ArrayList<Boolean>();
for (Expr expr : partitionExprs) {
if (expr instanceof SlotRef) {
partitionSlotNullables.add(findAllowNullforSlotRef(baseSchema, (SlotRef) expr));
} else if (expr instanceof FunctionCallExpr) {
partitionSlotNullables.add(Expr.isNullable(((FunctionCallExpr) expr).getFn(), expr.getChildren()));
} else {
throw new AnalysisException("Unknown partition expr type:" + expr.getExprName());
}
}

if (partition.getPartitionKeyDesc().getPartitionType() == PartitionKeyValueType.IN) {
List<List<PartitionValue>> inValues = partition.getPartitionKeyDesc().getInValues();
for (List<PartitionValue> item : inValues) {
checkNullityEqual(partitionSlotNullables, item);
}
} else if (partition.getPartitionKeyDesc().getPartitionType() == PartitionKeyValueType.LESS_THAN) {
// only upper
List<PartitionValue> upperValues = partition.getPartitionKeyDesc().getUpperValues();
checkNullityEqual(partitionSlotNullables, upperValues);
} else {
// fixed. upper and lower
List<PartitionValue> lowerValues = partition.getPartitionKeyDesc().getLowerValues();
List<PartitionValue> upperValues = partition.getPartitionKeyDesc().getUpperValues();
checkNullityEqual(partitionSlotNullables, lowerValues);
checkNullityEqual(partitionSlotNullables, upperValues);
}
}

private void checkLegalityofPartitionExprs(CreateTableStmt stmt, PartitionDesc partitionDesc)
throws AnalysisException {
for (Expr expr : partitionDesc.getPartitionExprs()) {
if (expr != null && expr instanceof FunctionCallExpr) { // test them
FunctionCallExpr func = (FunctionCallExpr) expr;
ArrayList<Expr> children = func.getChildren();
Type[] childTypes = new Type[children.size()];
for (int i = 0; i < children.size(); i++) {
if (children.get(i) instanceof LiteralExpr) {
childTypes[i] = children.get(i).getType();
} else if (children.get(i) instanceof SlotRef) {
childTypes[i] = getChildTypeByName(children.get(i).getExprName(), stmt);
} else {
throw new AnalysisException(String.format(
"partition expr %s has unrecognized parameter in slot %d", func.getExprName(), i));
}
}
Function fn = null;
try {
fn = func.getBuiltinFunction(func.getFnName().getFunction(), childTypes,
Function.CompareMode.IS_INDISTINGUISHABLE); // only for test
} catch (Exception e) {
throw new AnalysisException("partition expr " + func.getExprName() + " is illegal!");
}
if (fn == null) {
throw new AnalysisException("partition expr " + func.getExprName() + " is illegal!");
}
}
}
}

// Create olap table and related base index synchronously.
private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserException {
String tableName = stmt.getTableName();
Expand Down Expand Up @@ -2161,43 +2255,21 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx
// create partition info
PartitionDesc partitionDesc = stmt.getPartitionDesc();

// check legality of partiton exprs
ConnectContext ctx = ConnectContext.get();
Env env = Env.getCurrentEnv();

// check legality of partiton exprs.
if (ctx != null && env != null && partitionDesc != null && partitionDesc.getPartitionExprs() != null) {
for (Expr expr : partitionDesc.getPartitionExprs()) {
if (expr != null && expr instanceof FunctionCallExpr) { // test them
FunctionCallExpr func = (FunctionCallExpr) expr;
ArrayList<Expr> children = func.getChildren();
Type[] childTypes = new Type[children.size()];
for (int i = 0; i < children.size(); i++) {
if (children.get(i) instanceof LiteralExpr) {
childTypes[i] = children.get(i).getType();
} else if (children.get(i) instanceof SlotRef) {
childTypes[i] = getChildTypeByName(children.get(i).getExprName(), stmt);
} else {
throw new AnalysisException(String.format(
"partition expr %s has unrecognized parameter in slot %d", func.getExprName(), i));
}
}
Function fn = null;
try {
fn = func.getBuiltinFunction(func.getFnName().getFunction(), childTypes,
Function.CompareMode.IS_INDISTINGUISHABLE); // only for test
} catch (Exception e) {
throw new AnalysisException("partition expr " + func.getExprName() + " is illegal!");
}
if (fn == null) {
throw new AnalysisException("partition expr " + func.getExprName() + " is illegal!");
}
}
}
checkLegalityofPartitionExprs(stmt, partitionDesc);
}

PartitionInfo partitionInfo = null;
Map<String, Long> partitionNameToId = Maps.newHashMap();
if (partitionDesc != null) {
for (SinglePartitionDesc desc : partitionDesc.getSinglePartitionDescs()) {
// check legality of nullity of partition items.
checkPartitionNullity(baseSchema, partitionDesc, desc);

long partitionId = idGeneratorBuffer.getNextId();
partitionNameToId.put(desc.getPartitionName(), partitionId);
}
Expand Down
7 changes: 2 additions & 5 deletions ...-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateTableCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,8 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception {
LOG.debug("Nereids start to execute the create table command, query id: {}, tableName: {}",
ctx.queryId(), createTableInfo.getTableName());
}
try {
Env.getCurrentEnv().createTable(createTableStmt);
} catch (Exception e) {
throw new AnalysisException(e.getMessage(), e.getCause());
}

Env.getCurrentEnv().createTable(createTableStmt);
return;
}
LogicalPlan query = ctasQuery.get();
Expand Down
29 changes: 14 additions & 15 deletions fe/fe-core/src/test/java/org/apache/doris/catalog/CreateTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,17 +356,16 @@ public void testAbnormal() throws DdlException, ConfigException {

// single partition column with multi keys
ExceptionChecker
.expectThrowsWithMsg(IllegalArgumentException.class, "partition key desc list size[2] is not equal to partition column size[1]",
() -> createTable("create table test.tbl10\n"
+ "(k1 int not null, k2 varchar(128), k3 int, v1 int, v2 int)\n"
+ "partition by list(k1)\n"
+ "(\n"
+ "partition p1 values in (\"1\", \"3\", \"5\"),\n"
+ "partition p2 values in (\"2\", \"4\", \"6\"),\n"
+ "partition p3 values in ((\"7\", \"8\"))\n"
+ ")\n"
+ "distributed by hash(k2) buckets 1\n"
+ "properties('replication_num' = '1');"));
.expectThrowsWithMsg(AnalysisException.class,
"partition item's size out of partition columns: Index 1 out of bounds for length 1",
() -> createTable("create table test.tbl10\n"
+ "(k1 int not null, k2 varchar(128), k3 int, v1 int, v2 int)\n"
+ "partition by list(k1)\n" + "(\n"
+ "partition p1 values in (\"1\", \"3\", \"5\"),\n"
+ "partition p2 values in (\"2\", \"4\", \"6\"),\n"
+ "partition p3 values in ((\"7\", \"8\"))\n" + ")\n"
+ "distributed by hash(k2) buckets 1\n"
+ "properties('replication_num' = '1');"));

// multi partition columns with single key
ExceptionChecker
Expand All @@ -383,7 +382,7 @@ public void testAbnormal() throws DdlException, ConfigException {

// multi partition columns with multi keys
ExceptionChecker
.expectThrowsWithMsg(IllegalArgumentException.class, "partition key desc list size[3] is not equal to partition column size[2]",
.expectThrowsWithMsg(AnalysisException.class, "partition item's size out of partition columns: Index 2 out of bounds for length 2",
() -> createTable("create table test.tbl12\n"
+ "(k1 int not null, k2 varchar(128) not null, k3 int, v1 int, v2 int)\n"
+ "partition by list(k1, k2)\n"
Expand Down Expand Up @@ -906,8 +905,8 @@ public void testCreateTableWithMinLoadReplicaNum() throws Exception {

@Test
public void testCreateTableWithNerieds() throws Exception {
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.nereids.exceptions.AnalysisException.class,
"Failed to check min load replica num",
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.common.DdlException.class,
"Failed to check min load replica num",
() -> createTable("create table test.tbl_min_load_replica_num_2_nereids\n"
+ "(k1 int, k2 int)\n"
+ "duplicate key(k1)\n"
Expand Down Expand Up @@ -948,7 +947,7 @@ public void testCreateTableWithNerieds() throws Exception {
+ "distributed by hash(k1) buckets 10", true));

createDatabaseWithSql("create database db2 properties('replication_num' = '4')");
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.nereids.exceptions.AnalysisException.class,
ExceptionChecker.expectThrowsWithMsg(DdlException.class,
"replication num should be less than the number of available backends. "
+ "replication num is 4, available backend num is 3",
() -> createTable("create table db2.tbl_4_replica\n"
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/test/java/org/apache/doris/datasource/hive/HiveDDLAndDMLPlanTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public void testCreateAndDropWithSql() throws Exception {
+ "PROPERTIES (\n"
+ " 'location'='hdfs://loc/db/tbl',\n"
+ " 'file_format'='orc')";
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.nereids.exceptions.AnalysisException.class,
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.common.UserException.class,
"errCode = 2, detailMessage = errCode = 2,"
+ " detailMessage = Create hive bucket table need set enable_create_hive_bucket_table to true",
() -> createTable(createBucketedTableErr, true));
Expand Down
Loading

0 comments on commit e0af14c

Please sign in to comment.