This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 13780e4827d [fix](nereids)(create-table) fix bug that replication num
is not set when create table with no property (#25651)
13780e4827d is described below
commit 13780e4827db38494f2f3fb168f3195163a18fc1
Author: Mingyu Chen <[email protected]>
AuthorDate: Sat Oct 21 23:15:08 2023 +0800
[fix](nereids)(create-table) fix bug that replication num is not set when
create table with no property (#25651)
When executing create partitioned table with Nereids, and replication_num
property is not set,
the replication number will be 0, so the tablet will has no replica.
---
.../org/apache/doris/analysis/CreateTableStmt.java | 75 +-------------
.../apache/doris/common/util/PropertyAnalyzer.java | 72 ++++++++++++++
.../doris/nereids/parser/LogicalPlanBuilder.java | 11 ++-
.../trees/plans/commands/info/CreateTableInfo.java | 45 ++++++---
.../plans/commands/info/FixedRangePartition.java | 10 +-
.../trees/plans/commands/info/InPartition.java | 9 +-
.../plans/commands/info/LessThanPartition.java | 10 +-
.../plans/commands/info/PartitionDefinition.java | 22 ++++-
.../trees/plans/commands/info/StepPartition.java | 9 +-
.../org/apache/doris/catalog/CreateTableTest.java | 110 +++++++++++++--------
.../org/apache/doris/common/ExceptionChecker.java | 1 +
.../apache/doris/utframe/TestWithFeService.java | 37 +++++--
12 files changed, 240 insertions(+), 171 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
index a03884f5c6e..f56de294d98 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
@@ -20,13 +20,11 @@ package org.apache.doris.analysis;
import org.apache.doris.analysis.IndexDef.IndexType;
import org.apache.doris.catalog.AggregateType;
import org.apache.doris.catalog.Column;
-import org.apache.doris.catalog.DatabaseIf;
import org.apache.doris.catalog.DistributionInfo;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.Index;
import org.apache.doris.catalog.KeysType;
import org.apache.doris.catalog.PrimitiveType;
-import org.apache.doris.catalog.ReplicaAllocation;
import org.apache.doris.catalog.Type;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
@@ -41,7 +39,6 @@ import org.apache.doris.common.util.ParseUtil;
import org.apache.doris.common.util.PrintableMap;
import org.apache.doris.common.util.PropertyAnalyzer;
import org.apache.doris.common.util.Util;
-import org.apache.doris.datasource.CatalogIf;
import org.apache.doris.external.elasticsearch.EsUtil;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.ConnectContext;
@@ -51,7 +48,6 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -559,7 +555,8 @@ public class CreateTableStmt extends DdlStmt {
if (engineName.equals("olap")) {
// before analyzing partition, handle the replication allocation
info
- properties = rewriteReplicaAllocationProperties(properties);
+ properties = PropertyAnalyzer.rewriteReplicaAllocationProperties(
+ tableName.getCtl(), tableName.getDb(), properties);
// analyze partition
if (partitionDesc != null) {
if (partitionDesc instanceof ListPartitionDesc ||
partitionDesc instanceof RangePartitionDesc
@@ -650,74 +647,6 @@ public class CreateTableStmt extends DdlStmt {
}
}
- private Map<String, String> rewriteReplicaAllocationProperties(Map<String,
String> properties)
- throws AnalysisException {
- if (Config.force_olap_table_replication_num <= 0) {
- return rewriteReplicaAllocationPropertiesByDatabase(properties);
- }
- // if force_olap_table_replication_num is set, use this value to
rewrite the replication_num or
- // replication_allocation properties
- Map<String, String> newProperties = properties;
- if (newProperties == null) {
- newProperties = Maps.newHashMap();
- }
- boolean rewrite = false;
- if
(newProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM)) {
- newProperties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
- String.valueOf(Config.force_olap_table_replication_num));
- rewrite = true;
- }
- if
(newProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION))
{
-
newProperties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION,
- new ReplicaAllocation((short)
Config.force_olap_table_replication_num).toCreateStmt());
- rewrite = true;
- }
- if (!rewrite) {
- newProperties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
- String.valueOf(Config.force_olap_table_replication_num));
- }
- return newProperties;
- }
-
- private Map<String, String>
rewriteReplicaAllocationPropertiesByDatabase(Map<String, String> properties)
- throws AnalysisException {
- // if table contain `replication_allocation` or
`replication_allocation`,not need rewrite by db
- if (properties != null &&
(properties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION)
- ||
properties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM))) {
- return properties;
- }
- CatalogIf catalog =
Env.getCurrentEnv().getCatalogMgr().getCatalogNullable(tableName.getCtl());
- if (catalog == null) {
- return properties;
- }
- DatabaseIf db = catalog.getDbNullable(tableName.getDb());
- if (db == null) {
- return properties;
- }
- // if db not have properties,not need rewrite
- if (db.getDbProperties() == null) {
- return properties;
- }
- Map<String, String> dbProperties =
db.getDbProperties().getProperties();
- if (dbProperties == null) {
- return properties;
- }
- if (properties == null) {
- properties = Maps.newHashMap();
- }
- if
(dbProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION)
&& StringUtils
-
.isNotEmpty(dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION)))
{
- properties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION,
-
dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION));
- }
- if
(dbProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM) &&
StringUtils
-
.isNotEmpty(dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM))) {
- properties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
-
dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM));
- }
- return properties;
- }
-
private void analyzeEngineName() throws AnalysisException {
if (Strings.isNullOrEmpty(engineName)) {
engineName = "olap";
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
index a3f2202246f..5e037c2e3b8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
@@ -21,6 +21,7 @@ import org.apache.doris.analysis.DataSortInfo;
import org.apache.doris.analysis.DateLiteral;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.DataProperty;
+import org.apache.doris.catalog.DatabaseIf;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.EsResource;
import org.apache.doris.catalog.KeysType;
@@ -32,6 +33,7 @@ import org.apache.doris.catalog.Type;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
+import org.apache.doris.datasource.CatalogIf;
import org.apache.doris.datasource.CatalogMgr;
import org.apache.doris.policy.Policy;
import org.apache.doris.policy.StoragePolicy;
@@ -48,6 +50,7 @@ import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -1176,4 +1179,73 @@ public class PropertyAnalyzer {
}
}
}
+
+ public static Map<String, String> rewriteReplicaAllocationProperties(
+ String ctl, String db, Map<String, String> properties) {
+ if (Config.force_olap_table_replication_num <= 0) {
+ return rewriteReplicaAllocationPropertiesByDatabase(ctl, db,
properties);
+ }
+ // if force_olap_table_replication_num is set, use this value to
rewrite the replication_num or
+ // replication_allocation properties
+ Map<String, String> newProperties = properties;
+ if (newProperties == null) {
+ newProperties = Maps.newHashMap();
+ }
+ boolean rewrite = false;
+ if
(newProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM)) {
+ newProperties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
+ String.valueOf(Config.force_olap_table_replication_num));
+ rewrite = true;
+ }
+ if
(newProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION))
{
+
newProperties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION,
+ new ReplicaAllocation((short)
Config.force_olap_table_replication_num).toCreateStmt());
+ rewrite = true;
+ }
+ if (!rewrite) {
+ newProperties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
+ String.valueOf(Config.force_olap_table_replication_num));
+ }
+ return newProperties;
+ }
+
+ private static Map<String, String>
rewriteReplicaAllocationPropertiesByDatabase(
+ String ctl, String database, Map<String, String> properties) {
+ // if table contain `replication_allocation` or
`replication_allocation`,not need rewrite by db
+ if (properties != null &&
(properties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION)
+ ||
properties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM))) {
+ return properties;
+ }
+ CatalogIf catalog =
Env.getCurrentEnv().getCatalogMgr().getCatalogNullable(ctl);
+ if (catalog == null) {
+ return properties;
+ }
+ DatabaseIf db = catalog.getDbNullable(database);
+ if (db == null) {
+ return properties;
+ }
+ // if db not have properties,not need rewrite
+ if (db.getDbProperties() == null) {
+ return properties;
+ }
+ Map<String, String> dbProperties =
db.getDbProperties().getProperties();
+ if (dbProperties == null) {
+ return properties;
+ }
+ if (properties == null) {
+ properties = Maps.newHashMap();
+ }
+ if
(dbProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION)
&& StringUtils
+
.isNotEmpty(dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION)))
{
+ properties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION,
+
dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_ALLOCATION));
+ }
+ if
(dbProperties.containsKey(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM) &&
StringUtils
+
.isNotEmpty(dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM))) {
+ properties.put(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM,
+
dbProperties.get(PropertyAnalyzer.PROPERTIES_REPLICATION_NUM));
+ }
+ return properties;
+ }
+
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index 734a215888b..b817803f23c 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -1865,8 +1865,9 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
@Override
public LogicalPlan visitCreateTable(CreateTableContext ctx) {
+ String ctlName = null;
String dbName = null;
- String tableName;
+ String tableName = null;
List<String> nameParts = visitMultipartIdentifier(ctx.name);
// TODO: support catalog
if (nameParts.size() == 1) {
@@ -1874,8 +1875,12 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
} else if (nameParts.size() == 2) {
dbName = nameParts.get(0);
tableName = nameParts.get(1);
+ } else if (nameParts.size() == 3) {
+ ctlName = nameParts.get(0);
+ dbName = nameParts.get(1);
+ tableName = nameParts.get(2);
} else {
- throw new AnalysisException("nameParts in create table should be 1
or 2");
+ throw new AnalysisException("nameParts in create table should be
[ctl.][db.]tbl");
}
KeysType keysType = null;
if (ctx.DUPLICATE() != null) {
@@ -1906,6 +1911,7 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
}
return new CreateTableCommand(Optional.empty(), new
CreateTableInfo(
ctx.EXISTS() != null,
+ ctlName,
dbName,
tableName,
visitColumnDefs(ctx.columnDefs()),
@@ -1923,6 +1929,7 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
} else if (ctx.AS() != null) {
return new
CreateTableCommand(Optional.of(visitQuery(ctx.query())), new CreateTableInfo(
ctx.EXISTS() != null,
+ ctlName,
dbName,
tableName,
ctx.ctasCols != null ? visitIdentifierList(ctx.ctasCols) :
null,
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java
index f398c7b7d12..af11d4944cd 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java
@@ -30,17 +30,21 @@ import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.Index;
import org.apache.doris.catalog.KeysType;
+import org.apache.doris.catalog.PartitionType;
import org.apache.doris.catalog.Type;
import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.Config;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.FeNameFormat;
import org.apache.doris.common.util.PropertyAnalyzer;
+import org.apache.doris.datasource.InternalCatalog;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.util.Utils;
import org.apache.doris.qe.ConnectContext;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -57,6 +61,7 @@ import java.util.stream.Collectors;
*/
public class CreateTableInfo {
private final boolean ifNotExists;
+ private String ctlName;
private String dbName;
private final String tableName;
private List<ColumnDefinition> columns;
@@ -77,11 +82,13 @@ public class CreateTableInfo {
/**
* constructor for create table
*/
- public CreateTableInfo(boolean ifNotExists, String dbName, String
tableName, List<ColumnDefinition> columns,
- List<IndexDefinition> indexes, String engineName, KeysType
keysType, List<String> keys, String comment,
+ public CreateTableInfo(boolean ifNotExists, String ctlName, String dbName,
String tableName,
+ List<ColumnDefinition> columns, List<IndexDefinition> indexes,
String engineName,
+ KeysType keysType, List<String> keys, String comment,
String partitionType, List<String> partitionColumns,
List<PartitionDefinition> partitions,
DistributionDescriptor distribution, List<RollupDefinition>
rollups, Map<String, String> properties) {
this.ifNotExists = ifNotExists;
+ this.ctlName = ctlName;
this.dbName = dbName;
this.tableName = tableName;
this.ctasColumns = null;
@@ -102,11 +109,12 @@ public class CreateTableInfo {
/**
* constructor for create table as select
*/
- public CreateTableInfo(boolean ifNotExists, String dbName, String
tableName, List<String> cols,
+ public CreateTableInfo(boolean ifNotExists, String ctlName, String dbName,
String tableName, List<String> cols,
String engineName, KeysType keysType, List<String> keys, String
comment,
String partitionType, List<String> partitionColumns,
List<PartitionDefinition> partitions,
DistributionDescriptor distribution, List<RollupDefinition>
rollups, Map<String, String> properties) {
this.ifNotExists = ifNotExists;
+ this.ctlName = ctlName;
this.dbName = dbName;
this.tableName = tableName;
this.ctasColumns = cols;
@@ -128,6 +136,10 @@ public class CreateTableInfo {
return ctasColumns;
}
+ public String getCtlName() {
+ return ctlName;
+ }
+
public String getDbName() {
return dbName;
}
@@ -163,20 +175,30 @@ public class CreateTableInfo {
try {
FeNameFormat.checkTableName(tableName);
- if (dbName != null) {
- FeNameFormat.checkDbName(dbName);
- }
} catch (Exception e) {
throw new AnalysisException(e.getMessage(), e);
}
+ // analyze catalog name
+ if (Strings.isNullOrEmpty(ctlName)) {
+ if (ctx.getCurrentCatalog() != null) {
+ ctlName = ctx.getCurrentCatalog().getName();
+ } else {
+ ctlName = InternalCatalog.INTERNAL_CATALOG_NAME;
+ }
+ }
+
// analyze table name
- if (dbName == null) {
+ if (Strings.isNullOrEmpty(dbName)) {
dbName = ClusterNamespace.getFullName(ctx.getClusterName(),
ctx.getDatabase());
} else {
dbName = ClusterNamespace.getFullName(ctx.getClusterName(),
dbName);
}
+ Preconditions.checkState(!Strings.isNullOrEmpty(ctlName), "catalog
name is null or empty");
+ Preconditions.checkState(!Strings.isNullOrEmpty(dbName), "database
name is null or empty");
+ properties =
PropertyAnalyzer.rewriteReplicaAllocationProperties(ctlName, dbName,
properties);
+
boolean enableDuplicateWithoutKeysByDefault = false;
if (properties != null) {
try {
@@ -367,14 +389,15 @@ public class CreateTableInfo {
* check partitions types.
*/
private boolean checkPartitionsTypes() {
- if (partitionType.equalsIgnoreCase("RANGE")) {
+ if (partitionType.equalsIgnoreCase(PartitionType.RANGE.name())) {
if (partitions.stream().allMatch(p -> p instanceof StepPartition))
{
return true;
}
return partitions.stream().allMatch(p -> (p instanceof
LessThanPartition)
|| (p instanceof FixedRangePartition));
}
- return partitionType.equalsIgnoreCase("LIST") &&
partitions.stream().allMatch(p -> p instanceof InPartition);
+ return partitionType.equalsIgnoreCase(PartitionType.LIST.name())
+ && partitions.stream().allMatch(p -> p instanceof InPartition);
}
private void validatePartitionColumn(ColumnDefinition column,
ConnectContext ctx) {
@@ -395,7 +418,7 @@ public class CreateTableInfo {
if (!ctx.getSessionVariable().isAllowPartitionColumnNullable() &&
column.isNullable()) {
throw new AnalysisException("The partition column must be NOT
NULL");
}
- if (partitionType.equalsIgnoreCase("LIST") && column.isNullable()) {
+ if (partitionType.equalsIgnoreCase(PartitionType.LIST.name()) &&
column.isNullable()) {
throw new AnalysisException("The list partition column must be NOT
NULL");
}
}
@@ -415,7 +438,7 @@ public class CreateTableInfo {
List<AllPartitionDesc> partitionDescs = partitions.stream()
.map(PartitionDefinition::translateToCatalogStyle).collect(Collectors.toList());
try {
- if (partitionType.equals("RANGE")) {
+ if (partitionType.equals(PartitionType.RANGE.name())) {
partitionDesc = new RangePartitionDesc(partitionColumns,
partitionDescs);
} else {
partitionDesc = new ListPartitionDesc(partitionColumns,
partitionDescs);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/FixedRangePartition.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/FixedRangePartition.java
index 634f5c235dc..a1d74d20691 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/FixedRangePartition.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/FixedRangePartition.java
@@ -20,9 +20,6 @@ package org.apache.doris.nereids.trees.plans.commands.info;
import org.apache.doris.analysis.PartitionKeyDesc;
import org.apache.doris.analysis.PartitionValue;
import org.apache.doris.analysis.SinglePartitionDesc;
-import org.apache.doris.catalog.ReplicaAllocation;
-import org.apache.doris.common.util.PropertyAnalyzer;
-import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.types.DataType;
@@ -39,7 +36,6 @@ public class FixedRangePartition extends PartitionDefinition {
private final String partitionName;
private List<Expression> lowerBounds;
private List<Expression> upperBounds;
- private ReplicaAllocation replicaAllocation =
ReplicaAllocation.DEFAULT_ALLOCATION;
public FixedRangePartition(String partitionName, List<Expression>
lowerBounds, List<Expression> upperBounds) {
this.partitionName = partitionName;
@@ -49,11 +45,7 @@ public class FixedRangePartition extends PartitionDefinition
{
@Override
public void validate(Map<String, String> properties) {
- try {
- replicaAllocation =
PropertyAnalyzer.analyzeReplicaAllocation(properties, "");
- } catch (Exception e) {
- throw new AnalysisException(e.getMessage(), e.getCause());
- }
+ super.validate(properties);
final DataType type = partitionTypes.get(0);
lowerBounds = lowerBounds.stream().map(e ->
e.castTo(type)).collect(Collectors.toList());
upperBounds = upperBounds.stream().map(e ->
e.castTo(type)).collect(Collectors.toList());
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/InPartition.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/InPartition.java
index 900f521cabc..84821d99212 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/InPartition.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/InPartition.java
@@ -21,8 +21,6 @@ import org.apache.doris.analysis.AllPartitionDesc;
import org.apache.doris.analysis.PartitionKeyDesc;
import org.apache.doris.analysis.PartitionValue;
import org.apache.doris.analysis.SinglePartitionDesc;
-import org.apache.doris.catalog.ReplicaAllocation;
-import org.apache.doris.common.util.PropertyAnalyzer;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
@@ -38,7 +36,6 @@ import java.util.stream.Collectors;
public class InPartition extends PartitionDefinition {
private final String partitionName;
private final List<List<Expression>> values;
- private ReplicaAllocation replicaAllocation =
ReplicaAllocation.DEFAULT_ALLOCATION;
public InPartition(String partitionName, List<List<Expression>> values) {
this.partitionName = partitionName;
@@ -47,11 +44,7 @@ public class InPartition extends PartitionDefinition {
@Override
public void validate(Map<String, String> properties) {
- try {
- replicaAllocation =
PropertyAnalyzer.analyzeReplicaAllocation(properties, "");
- } catch (Exception e) {
- throw new AnalysisException(e.getMessage(), e.getCause());
- }
+ super.validate(properties);
if (values.stream().anyMatch(l ->
l.stream().anyMatch(MaxValue.class::isInstance))) {
throw new AnalysisException("MAXVALUE cannot be used in 'in
partition'");
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java
index 2537c5d1a1a..11a6f4f50da 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java
@@ -20,9 +20,6 @@ package org.apache.doris.nereids.trees.plans.commands.info;
import org.apache.doris.analysis.PartitionKeyDesc;
import org.apache.doris.analysis.PartitionValue;
import org.apache.doris.analysis.SinglePartitionDesc;
-import org.apache.doris.catalog.ReplicaAllocation;
-import org.apache.doris.common.util.PropertyAnalyzer;
-import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import com.google.common.collect.Maps;
@@ -37,7 +34,6 @@ import java.util.stream.Collectors;
public class LessThanPartition extends PartitionDefinition {
private final String partitionName;
private final List<Expression> values;
- private ReplicaAllocation replicaAllocation =
ReplicaAllocation.DEFAULT_ALLOCATION;
public LessThanPartition(String partitionName, List<Expression> values) {
this.partitionName = partitionName;
@@ -46,11 +42,7 @@ public class LessThanPartition extends PartitionDefinition {
@Override
public void validate(Map<String, String> properties) {
- try {
- replicaAllocation =
PropertyAnalyzer.analyzeReplicaAllocation(properties, "");
- } catch (Exception e) {
- throw new AnalysisException(e.getMessage(), e.getCause());
- }
+ super.validate(properties);
}
public String getPartitionName() {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java
index a6edda1b83d..2d89ed94909 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java
@@ -19,6 +19,8 @@ package org.apache.doris.nereids.trees.plans.commands.info;
import org.apache.doris.analysis.AllPartitionDesc;
import org.apache.doris.analysis.PartitionValue;
+import org.apache.doris.catalog.ReplicaAllocation;
+import org.apache.doris.common.util.PropertyAnalyzer;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
@@ -34,16 +36,30 @@ import java.util.Map;
*/
public abstract class PartitionDefinition {
protected List<DataType> partitionTypes;
- protected Map<String, String> propreties;
+ protected Map<String, String> properties;
+ protected ReplicaAllocation replicaAllocation =
ReplicaAllocation.DEFAULT_ALLOCATION;
public PartitionDefinition withProperties(Map<String, String> properties) {
- this.propreties = properties;
+ this.properties = properties;
return this;
}
public abstract AllPartitionDesc translateToCatalogStyle();
- public abstract void validate(Map<String, String> properties);
+ /**
+ * Validate the properties.
+ * Derived class can override this method to do more validation.
+ */
+ public void validate(Map<String, String> properties) {
+ try {
+ replicaAllocation =
PropertyAnalyzer.analyzeReplicaAllocation(properties, "");
+ if (replicaAllocation.isNotSet()) {
+ replicaAllocation = ReplicaAllocation.DEFAULT_ALLOCATION;
+ }
+ } catch (Exception e) {
+ throw new AnalysisException(e.getMessage(), e.getCause());
+ }
+ }
/**
* get partition name
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/StepPartition.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/StepPartition.java
index 6588ad1dc35..a02013d3770 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/StepPartition.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/StepPartition.java
@@ -20,8 +20,6 @@ package org.apache.doris.nereids.trees.plans.commands.info;
import org.apache.doris.analysis.MultiPartitionDesc;
import org.apache.doris.analysis.PartitionKeyDesc;
import org.apache.doris.analysis.PartitionValue;
-import org.apache.doris.catalog.ReplicaAllocation;
-import org.apache.doris.common.util.PropertyAnalyzer;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
@@ -39,7 +37,6 @@ public class StepPartition extends PartitionDefinition {
private final List<Expression> toExpression;
private final long unit;
private final String unitString;
- private ReplicaAllocation replicaAllocation =
ReplicaAllocation.DEFAULT_ALLOCATION;
public StepPartition(List<Expression> fromExpression, List<Expression>
toExpression, long unit,
String unitString) {
@@ -51,15 +48,11 @@ public class StepPartition extends PartitionDefinition {
@Override
public void validate(Map<String, String> properties) {
+ super.validate(properties);
if (fromExpression.stream().anyMatch(MaxValue.class::isInstance)
|| toExpression.stream().anyMatch(MaxValue.class::isInstance))
{
throw new AnalysisException("MAXVALUE cannot be used in step
partition");
}
- try {
- replicaAllocation =
PropertyAnalyzer.analyzeReplicaAllocation(properties, "");
- } catch (Exception e) {
- throw new AnalysisException(e.getMessage(), e.getCause());
- }
}
/**
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateTableTest.java
b/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateTableTest.java
index f8b37973214..3bf6af35c2a 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateTableTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/CreateTableTest.java
@@ -17,9 +17,6 @@
package org.apache.doris.catalog;
-import org.apache.doris.analysis.AlterTableStmt;
-import org.apache.doris.analysis.CreateDbStmt;
-import org.apache.doris.analysis.CreateTableStmt;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.ConfigBase;
@@ -27,51 +24,27 @@ import org.apache.doris.common.ConfigException;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
-import org.apache.doris.qe.ConnectContext;
-import org.apache.doris.utframe.UtFrameUtils;
+import org.apache.doris.utframe.TestWithFeService;
-import org.junit.AfterClass;
import org.junit.Assert;
-import org.junit.BeforeClass;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import java.io.File;
import java.util.HashSet;
import java.util.Set;
import java.util.UUID;
-public class CreateTableTest {
+public class CreateTableTest extends TestWithFeService {
private static String runningDir = "fe/mocked/CreateTableTest2/" +
UUID.randomUUID().toString() + "/";
- private static ConnectContext connectContext;
-
- @BeforeClass
- public static void beforeClass() throws Exception {
- Config.disable_storage_medium_check = true;
- UtFrameUtils.createDorisClusterWithMultiTag(runningDir, 3);
-
- // create connect context
- connectContext = UtFrameUtils.createDefaultCtx();
- // create database
- String createDbStmtStr = "create database test;";
- CreateDbStmt createDbStmt = (CreateDbStmt)
UtFrameUtils.parseAndAnalyzeStmt(createDbStmtStr, connectContext);
- Env.getCurrentEnv().createDb(createDbStmt);
+ @Override
+ protected int backendNum() {
+ return 3;
}
- @AfterClass
- public static void tearDown() {
- File file = new File(runningDir);
- file.delete();
- }
-
- private static void createTable(String sql) throws Exception {
- CreateTableStmt createTableStmt = (CreateTableStmt)
UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext);
- Env.getCurrentEnv().createTable(createTableStmt);
- }
-
- private static void alterTable(String sql) throws Exception {
- AlterTableStmt alterTableStmt = (AlterTableStmt)
UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext);
- Env.getCurrentEnv().alterTable(alterTableStmt);
+ @Override
+ protected void runBeforeAll() throws Exception {
+ Config.allow_replica_on_same_host = true;
+ createDatabase("test");
}
@Test
@@ -761,7 +734,7 @@ public class CreateTableTest {
// can still set replication_num manually.
ExceptionChecker.expectThrowsWithMsg(UserException.class, "Failed
to find enough host with tag",
() -> {
- alterTable("alter table test.test_replica modify
partition p1 set ('replication_num' = '4')");
+ alterTableSync("alter table test.test_replica modify
partition p1 set ('replication_num' = '4')");
});
Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException("default_cluster:test");
@@ -792,17 +765,17 @@ public class CreateTableTest {
Assert.assertEquals(2, (int)
tbl1.getDefaultReplicaAllocation().getTotalReplicaNum());
ExceptionChecker.expectThrowsNoException(
- () -> alterTable("alter table
test.tbl_min_load_replica_num_1\n"
+ () -> alterTableSync("alter table
test.tbl_min_load_replica_num_1\n"
+ " set ( 'min_load_replica_num' = '2');"));
Assert.assertEquals(2, tbl1.getMinLoadReplicaNum());
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Failed to
check min load replica num",
- () -> alterTable("alter table
test.tbl_min_load_replica_num_1\n"
+ () -> alterTableSync("alter table
test.tbl_min_load_replica_num_1\n"
+ " set ( 'min_load_replica_num' = '3');"));
Assert.assertEquals(2, tbl1.getMinLoadReplicaNum());
ExceptionChecker.expectThrowsWithMsg(DdlException.class,
"min_load_replica_num should > 0 or =-1",
- () -> alterTable("alter table
test.tbl_min_load_replica_num_1\n"
+ () -> alterTableSync("alter table
test.tbl_min_load_replica_num_1\n"
+ " set ( 'min_load_replica_num' = '-3');"));
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Failed to
check min load replica num",
@@ -881,7 +854,7 @@ public class CreateTableTest {
+ ");\n"));
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Failed to
check min load replica num",
- () -> alterTable("alter table
test.tbl_min_load_replica_num_6\n"
+ () -> alterTableSync("alter table
test.tbl_min_load_replica_num_6\n"
+ " set ( 'min_load_replica_num' = '3');"));
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Failed to
check min load replica num",
@@ -905,4 +878,57 @@ public class CreateTableTest {
+ "\"dynamic_partition.start\" = \"-3\"\n"
+ ");\n"));
}
+
+ @Test
+ public void testCreateTableWithNerieds() throws Exception {
+
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.nereids.exceptions.AnalysisException.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"
+ + "distributed by hash(k1) buckets 1\n"
+ + "properties(\n"
+ + " 'replication_num' = '2',\n"
+ + " 'min_load_replica_num' = '3'\n"
+ + ");", true));
+
+ ExceptionChecker.expectThrowsNoException(
+ () -> createTable("create table test.tbl_no_properties\n"
+ + "(k1 int, k2 int)\n"
+ + "duplicate key(k1)\n"
+ + "distributed by hash(k1) buckets 1", true));
+
+ ExceptionChecker.expectThrowsNoException(
+ () -> createTable("create table
test.tbl_range_part_no_properties\n"
+ + "(k1 int not null, k2 int)\n"
+ + "partition by range(k1)\n"
+ + "(partition p1 values less than ('100'),"
+ + "partition p2 values less than ('200'))\n"
+ + "distributed by hash(k1) buckets 1", true));
+
+ ExceptionChecker.expectThrowsNoException(
+ () -> createTable("create table
test.tbl_in_part_no_properties\n"
+ + "(k1 int not null, k2 int)\n"
+ + "partition by list(k1)\n"
+ + "(partition p1 values in ('100'),"
+ + "partition p2 values in ('200'))\n"
+ + "distributed by hash(k1) buckets 3", true));
+
+ ExceptionChecker.expectThrowsNoException(
+ () -> createTable("create table
test.tbl_fixed_part_no_properties\n"
+ + "(k1 int not null, k2 int)\n"
+ + "partition by range(k1)\n"
+ + "(partition p1 values [('100'),('200')),"
+ + "partition p2 values [('200'),('300')))\n"
+ + "distributed by hash(k1) buckets 10", true));
+
+ createDatabaseWithSql("create database db2
properties('replication_num' = '4')");
+
ExceptionChecker.expectThrowsWithMsg(org.apache.doris.nereids.exceptions.AnalysisException.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"
+ + "(k1 int, k2 int)\n"
+ + "duplicate key(k1)\n"
+ + "distributed by hash(k1) buckets 1\n", true));
+ }
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/common/ExceptionChecker.java
b/fe/fe-core/src/test/java/org/apache/doris/common/ExceptionChecker.java
index 26d5d1dc47a..5353cfd0274 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/common/ExceptionChecker.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/common/ExceptionChecker.java
@@ -34,6 +34,7 @@ public class ExceptionChecker {
try {
runnable.run();
} catch (Throwable e) {
+ e.printStackTrace();
throw new AssertionFailedError(e.getMessage());
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
b/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
index 5237909dd46..160bdee462e 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
@@ -59,8 +59,10 @@ import org.apache.doris.common.util.SqlParserUtils;
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.StatementContext;
import org.apache.doris.nereids.glue.LogicalPlanAdapter;
+import org.apache.doris.nereids.parser.NereidsParser;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator;
+import org.apache.doris.nereids.trees.plans.commands.CreateTableCommand;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.util.MemoTestUtils;
import org.apache.doris.planner.Planner;
@@ -101,7 +103,6 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Comparator;
-import java.util.ConcurrentModificationException;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -562,6 +563,11 @@ public abstract class TestWithFeService {
Env.getCurrentEnv().createDb(createDbStmt);
}
+ public void createDatabaseWithSql(String createDbSql) throws Exception {
+ CreateDbStmt createDbStmt = (CreateDbStmt)
parseAndAnalyzeStmt(createDbSql);
+ Env.getCurrentEnv().createDb(createDbStmt);
+ }
+
public void dropDatabase(String db) throws Exception {
String createDbStmtStr = "DROP DATABASE " + db;
DropDbStmt createDbStmt = (DropDbStmt)
parseAndAnalyzeStmt(createDbStmtStr);
@@ -597,10 +603,14 @@ public abstract class TestWithFeService {
}
public void createTable(String sql) throws Exception {
+ createTable(sql, false);
+ }
+
+ public void createTable(String sql, boolean enableNerieds) throws
Exception {
try {
Config.enable_odbc_table = true;
- createTables(sql);
- } catch (ConcurrentModificationException e) {
+ createTables(enableNerieds, sql);
+ } catch (Exception e) {
e.printStackTrace();
throw e;
}
@@ -624,9 +634,24 @@ public abstract class TestWithFeService {
}
public void createTables(String... sqls) throws Exception {
- for (String sql : sqls) {
- CreateTableStmt stmt = (CreateTableStmt) parseAndAnalyzeStmt(sql);
- Env.getCurrentEnv().createTable(stmt);
+ createTables(false, sqls);
+ }
+
+ public void createTables(boolean enableNereids, String... sqls) throws
Exception {
+ if (enableNereids) {
+ for (String sql : sqls) {
+ NereidsParser nereidsParser = new NereidsParser();
+ LogicalPlan parsed = nereidsParser.parseSingle(sql);
+ StmtExecutor stmtExecutor = new StmtExecutor(connectContext,
sql);
+ if (parsed instanceof CreateTableCommand) {
+ ((CreateTableCommand) parsed).run(connectContext,
stmtExecutor);
+ }
+ }
+ } else {
+ for (String sql : sqls) {
+ CreateTableStmt stmt = (CreateTableStmt)
parseAndAnalyzeStmt(sql);
+ Env.getCurrentEnv().createTable(stmt);
+ }
}
updateReplicaPathHash();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
