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 8759bce4266 [fix](stmt):fix CreateTableStmt toSql placed comment in
wrong place (#27504)
8759bce4266 is described below
commit 8759bce42664e74cfbcfec4e2a934217d4ae61a8
Author: htyoung <[email protected]>
AuthorDate: Thu Dec 21 09:57:20 2023 +0800
[fix](stmt):fix CreateTableStmt toSql placed comment in wrong place (#27504)
Issue Number: close #27474
Co-authored-by: tongyang.han <[email protected]>
---
.../org/apache/doris/analysis/CreateTableStmt.java | 159 ++++++++-------------
.../apache/doris/analysis/CreateTableStmtTest.java | 62 ++++++++
2 files changed, 123 insertions(+), 98 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 3bd12b64eef..210fa60857f 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
@@ -85,7 +85,6 @@ public class CreateTableStmt extends DdlStmt {
// set in analyze
private List<Column> columns = Lists.newArrayList();
-
private List<Index> indexes = Lists.newArrayList();
static {
@@ -130,48 +129,26 @@ public class CreateTableStmt extends DdlStmt {
columnDefs = Lists.newArrayList();
}
- public CreateTableStmt(boolean ifNotExists,
- boolean isExternal,
- TableName tableName,
- List<ColumnDef> columnDefinitions,
- String engineName,
- KeysDesc keysDesc,
- PartitionDesc partitionDesc,
- DistributionDesc distributionDesc,
- Map<String, String> properties,
- Map<String, String> extProperties,
+ public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName
tableName,
+ List<ColumnDef> columnDefinitions, String engineName, KeysDesc
keysDesc, PartitionDesc partitionDesc,
+ DistributionDesc distributionDesc, Map<String, String> properties,
Map<String, String> extProperties,
String comment) {
this(ifNotExists, isExternal, tableName, columnDefinitions, null,
engineName, keysDesc, partitionDesc,
distributionDesc, properties, extProperties, comment, null);
}
- public CreateTableStmt(boolean ifNotExists,
- boolean isExternal,
- TableName tableName,
- List<ColumnDef> columnDefinitions,
- String engineName,
- KeysDesc keysDesc,
- PartitionDesc partitionDesc,
- DistributionDesc distributionDesc,
- Map<String, String> properties,
- Map<String, String> extProperties,
+ public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName
tableName,
+ List<ColumnDef> columnDefinitions, String engineName, KeysDesc
keysDesc, PartitionDesc partitionDesc,
+ DistributionDesc distributionDesc, Map<String, String> properties,
Map<String, String> extProperties,
String comment, List<AlterClause> ops) {
this(ifNotExists, isExternal, tableName, columnDefinitions, null,
engineName, keysDesc, partitionDesc,
distributionDesc, properties, extProperties, comment, ops);
}
- public CreateTableStmt(boolean ifNotExists,
- boolean isExternal,
- TableName tableName,
- List<ColumnDef> columnDefinitions,
- List<IndexDef> indexDefs,
- String engineName,
- KeysDesc keysDesc,
- PartitionDesc partitionDesc,
- DistributionDesc distributionDesc,
- Map<String, String> properties,
- Map<String, String> extProperties,
- String comment, List<AlterClause> rollupAlterClauseList) {
+ public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName
tableName,
+ List<ColumnDef> columnDefinitions, List<IndexDef> indexDefs,
String engineName, KeysDesc keysDesc,
+ PartitionDesc partitionDesc, DistributionDesc distributionDesc,
Map<String, String> properties,
+ Map<String, String> extProperties, String comment,
List<AlterClause> rollupAlterClauseList) {
this.tableName = tableName;
if (columnDefinitions == null) {
this.columnDefs = Lists.newArrayList();
@@ -198,20 +175,10 @@ public class CreateTableStmt extends DdlStmt {
}
// for Nereids
- public CreateTableStmt(boolean ifNotExists,
- boolean isExternal,
- TableName tableName,
- List<Column> columns,
- List<Index> indexes,
- String engineName,
- KeysDesc keysDesc,
- PartitionDesc partitionDesc,
- DistributionDesc distributionDesc,
- Map<String, String> properties,
- Map<String, String> extProperties,
- String comment,
- List<AlterClause> rollupAlterClauseList,
- Void unused) {
+ public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName
tableName, List<Column> columns,
+ List<Index> indexes, String engineName, KeysDesc keysDesc,
PartitionDesc partitionDesc,
+ DistributionDesc distributionDesc, Map<String, String> properties,
Map<String, String> extProperties,
+ String comment, List<AlterClause> rollupAlterClauseList, Void
unused) {
this.ifNotExists = ifNotExists;
this.isExternal = isExternal;
this.tableName = tableName;
@@ -308,8 +275,8 @@ public class CreateTableStmt extends DdlStmt {
}
@Override
- public void analyze(Analyzer analyzer) throws UserException,
AnalysisException {
- if (Strings.isNullOrEmpty(engineName) ||
engineName.equalsIgnoreCase("olap")) {
+ public void analyze(Analyzer analyzer) throws UserException {
+ if (Strings.isNullOrEmpty(engineName) ||
engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
this.properties = maybeRewriteByAutoBucket(distributionDesc,
properties);
}
@@ -319,8 +286,8 @@ public class CreateTableStmt extends DdlStmt {
// disallow external catalog
Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
- if
(!Env.getCurrentEnv().getAccessManager().checkTblPriv(ConnectContext.get(),
tableName.getDb(),
- tableName.getTbl(), PrivPredicate.CREATE)) {
+ if (!Env.getCurrentEnv().getAccessManager()
+ .checkTblPriv(ConnectContext.get(), tableName.getDb(),
tableName.getTbl(), PrivPredicate.CREATE)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR,
"CREATE");
}
@@ -328,10 +295,10 @@ public class CreateTableStmt extends DdlStmt {
boolean enableDuplicateWithoutKeysByDefault = false;
if (properties != null) {
- enableDuplicateWithoutKeysByDefault =
-
PropertyAnalyzer.analyzeEnableDuplicateWithoutKeysByDefault(properties);
+ enableDuplicateWithoutKeysByDefault =
PropertyAnalyzer.analyzeEnableDuplicateWithoutKeysByDefault(
+ properties);
}
- //pre-block creation with column type ALL
+ // pre-block creation with column type ALL
for (ColumnDef columnDef : columnDefs) {
if (Objects.equals(columnDef.getType(), Type.ALL)) {
throw new AnalysisException("Disable to create table with
`ALL` type columns.");
@@ -340,14 +307,14 @@ public class CreateTableStmt extends DdlStmt {
throw new AnalysisException("Disable to create table with
`DATE` type columns, please use `DATEV2`.");
}
if (Objects.equals(columnDef.getType(), Type.DECIMALV2) &&
Config.disable_decimalv2) {
- throw new AnalysisException("Disable to create table with
`DECIMAL` type columns,"
- + "please use `DECIMALV3`.");
+ throw new AnalysisException(
+ "Disable to create table with `DECIMAL` type columns,"
+ "please use `DECIMALV3`.");
}
}
boolean enableUniqueKeyMergeOnWrite = false;
// analyze key desc
- if (engineName.equalsIgnoreCase("olap")) {
+ if (engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
// olap table
if (keysDesc == null) {
List<String> keysColumnNames = Lists.newArrayList();
@@ -361,9 +328,9 @@ public class CreateTableStmt extends DdlStmt {
}
if (hasAggregate) {
for (ColumnDef columnDef : columnDefs) {
- if (columnDef.getAggregateType() == null
- &&
!columnDef.getType().isScalarType(PrimitiveType.STRING)
- &&
!columnDef.getType().isScalarType(PrimitiveType.JSONB)) {
+ if (columnDef.getAggregateType() == null &&
!columnDef.getType()
+ .isScalarType(PrimitiveType.STRING) &&
!columnDef.getType()
+ .isScalarType(PrimitiveType.JSONB)) {
keysColumnNames.add(columnDef.getName());
}
}
@@ -374,8 +341,8 @@ public class CreateTableStmt extends DdlStmt {
keyLength += columnDef.getType().getIndexSize();
if (keysColumnNames.size() >=
FeConstants.shortkey_max_column_count
|| keyLength >
FeConstants.shortkey_maxsize_bytes) {
- if (keysColumnNames.size() == 0
- &&
columnDef.getType().getPrimitiveType().isCharFamily()) {
+ if (keysColumnNames.isEmpty() &&
columnDef.getType().getPrimitiveType()
+ .isCharFamily()) {
keysColumnNames.add(columnDef.getName());
}
break;
@@ -411,7 +378,7 @@ public class CreateTableStmt extends DdlStmt {
} else {
if (enableDuplicateWithoutKeysByDefault) {
throw new AnalysisException("table property
'enable_duplicate_without_keys_by_default' only can"
- + " set 'true' when create olap table by
default.");
+ + " set 'true' when create olap table by
default.");
}
}
@@ -463,13 +430,11 @@ public class CreateTableStmt extends DdlStmt {
}
// analyze column def
- if (!(engineName.equals("elasticsearch"))
- && (columnDefs == null || columnDefs.isEmpty())) {
+ if (!(engineName.equalsIgnoreCase("elasticsearch")) && (columnDefs ==
null || columnDefs.isEmpty())) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLE_MUST_HAVE_COLUMNS);
}
// add a hidden column as delete flag for unique table
- if (Config.enable_batch_delete_by_default
- && keysDesc != null
+ if (Config.enable_batch_delete_by_default && keysDesc != null
&& keysDesc.getKeysType() == KeysType.UNIQUE_KEYS) {
if (enableUniqueKeyMergeOnWrite) {
columnDefs.add(ColumnDef.newDeleteSignColumnDef(AggregateType.NONE));
@@ -502,14 +467,14 @@ public class CreateTableStmt extends DdlStmt {
}
Set<String> columnSet = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
for (ColumnDef columnDef : columnDefs) {
- columnDef.analyze(engineName.equals("olap"));
+
columnDef.analyze(engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME));
- if (columnDef.getType().isComplexType() &&
engineName.equals("olap")) {
- if (columnDef.getAggregateType() != null
- && columnDef.getAggregateType() != AggregateType.NONE
+ if (columnDef.getType().isComplexType() &&
engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
+ if (columnDef.getAggregateType() != null &&
columnDef.getAggregateType() != AggregateType.NONE
&& columnDef.getAggregateType() !=
AggregateType.REPLACE) {
- throw new
AnalysisException(columnDef.getType().getPrimitiveType()
- + " column can't support aggregation " +
columnDef.getAggregateType());
+ throw new AnalysisException(
+ columnDef.getType().getPrimitiveType() + " column
can't support aggregation "
+ + columnDef.getAggregateType());
}
if (columnDef.isKey()) {
throw new
AnalysisException(columnDef.getType().getPrimitiveType()
@@ -526,17 +491,17 @@ public class CreateTableStmt extends DdlStmt {
}
}
- if (engineName.equals("olap")) {
+ if (engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
// before analyzing partition, handle the replication allocation
info
- properties = PropertyAnalyzer.rewriteReplicaAllocationProperties(
- tableName.getCtl(), tableName.getDb(), properties);
+ properties =
PropertyAnalyzer.rewriteReplicaAllocationProperties(tableName.getCtl(),
tableName.getDb(),
+ properties);
// analyze partition
if (partitionDesc != null) {
if (partitionDesc instanceof ListPartitionDesc ||
partitionDesc instanceof RangePartitionDesc) {
partitionDesc.analyze(columnDefs, properties);
} else {
- throw new AnalysisException("Currently only support range"
- + " and list partition with engine type olap");
+ throw new AnalysisException(
+ "Currently only support range" + " and list
partition with engine type olap");
}
}
@@ -553,10 +518,10 @@ public class CreateTableStmt extends DdlStmt {
for (ColumnDef columnDef : columnDefs) {
if (columnDef.getAggregateType() ==
AggregateType.REPLACE
|| columnDef.getAggregateType() ==
AggregateType.REPLACE_IF_NOT_NULL) {
- throw new AnalysisException("Create aggregate keys
table with value columns of which"
- + " aggregate type is " +
columnDef.getAggregateType()
- + " should not contain random"
- + " distribution desc");
+ throw new AnalysisException(
+ "Create aggregate keys table with value
columns of which" + " aggregate type is "
+ + columnDef.getAggregateType() + "
should not contain random"
+ + " distribution desc");
}
}
}
@@ -565,8 +530,8 @@ public class CreateTableStmt extends DdlStmt {
EsUtil.analyzePartitionAndDistributionDesc(partitionDesc,
distributionDesc);
} else {
if (partitionDesc != null || distributionDesc != null) {
- throw new AnalysisException("Create " + engineName
- + " table should not contain partition or distribution
desc");
+ throw new AnalysisException(
+ "Create " + engineName + " table should not contain
partition or distribution desc");
}
}
@@ -586,7 +551,7 @@ public class CreateTableStmt extends DdlStmt {
for (IndexDef indexDef : indexDefs) {
indexDef.analyze();
- if (!engineName.equalsIgnoreCase("olap")) {
+ if (!engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
throw new AnalysisException("index only support in olap
engine at current version.");
}
for (String indexColName : indexDef.getColumns()) {
@@ -599,13 +564,11 @@ public class CreateTableStmt extends DdlStmt {
}
}
if (!found) {
- throw new AnalysisException("Column does not exist in
table. invalid column: "
- + indexColName);
+ throw new AnalysisException("Column does not exist in
table. invalid column: " + indexColName);
}
}
- indexes.add(new Index(Env.getCurrentEnv().getNextId(),
indexDef.getIndexName(),
- indexDef.getColumns(), indexDef.getIndexType(),
- indexDef.getProperties(), indexDef.getComment()));
+ indexes.add(new Index(Env.getCurrentEnv().getNextId(),
indexDef.getIndexName(), indexDef.getColumns(),
+ indexDef.getIndexType(), indexDef.getProperties(),
indexDef.getComment()));
distinct.add(indexDef.getIndexName());
distinctCol.add(Pair.of(indexDef.getIndexType(),
indexDef.getColumns().stream().map(String::toUpperCase).collect(Collectors.toList())));
@@ -687,12 +650,16 @@ public class CreateTableStmt extends DdlStmt {
}
}
sb.append("\n)");
- sb.append(" ENGINE = ").append(engineName);
+ sb.append(" ENGINE = ").append(engineName.toLowerCase());
if (keysDesc != null) {
sb.append("\n").append(keysDesc.toSql());
}
+ if (!Strings.isNullOrEmpty(comment)) {
+ sb.append("\nCOMMENT \"").append(comment).append("\"");
+ }
+
if (partitionDesc != null) {
sb.append("\n").append(partitionDesc.toSql());
}
@@ -701,7 +668,7 @@ public class CreateTableStmt extends DdlStmt {
sb.append("\n").append(distributionDesc.toSql());
}
- if (rollupAlterClauseList != null && rollupAlterClauseList.size() !=
0) {
+ if (rollupAlterClauseList != null && !rollupAlterClauseList.isEmpty())
{
sb.append("\n rollup(");
StringBuilder opsSb = new StringBuilder();
for (int i = 0; i < rollupAlterClauseList.size(); i++) {
@@ -719,20 +686,16 @@ public class CreateTableStmt extends DdlStmt {
// which is implemented in Catalog.getDdlStmt()
if (properties != null && !properties.isEmpty()) {
sb.append("\nPROPERTIES (");
- sb.append(new PrintableMap<String, String>(properties, " = ",
true, true, true));
+ sb.append(new PrintableMap<>(properties, " = ", true, true, true));
sb.append(")");
}
if (extProperties != null && !extProperties.isEmpty()) {
sb.append("\n").append(engineName.toUpperCase()).append("
PROPERTIES (");
- sb.append(new PrintableMap<String, String>(extProperties, " = ",
true, true, true));
+ sb.append(new PrintableMap<>(extProperties, " = ", true, true,
true));
sb.append(")");
}
- if (!Strings.isNullOrEmpty(comment)) {
- sb.append("\nCOMMENT \"").append(comment).append("\"");
- }
-
return sb.toString();
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java
b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java
index 30da4033976..ccd865dd6d4 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/analysis/CreateTableStmtTest.java
@@ -456,4 +456,66 @@ public class CreateTableStmtTest {
Assert.assertEquals(createTableStmt.toSql(), createTableSql);
}
+
+ @Test
+ public void testToSqlWithComment() {
+ List<ColumnDef> columnDefs = new ArrayList<>();
+ columnDefs.add(new ColumnDef("a",
TypeDef.create(PrimitiveType.BIGINT), false));
+ columnDefs.add(new ColumnDef("b", TypeDef.create(PrimitiveType.INT),
false));
+ String engineName = "olap";
+ ArrayList<String> aggKeys = Lists.newArrayList("a");
+ KeysDesc keysDesc = new KeysDesc(KeysType.AGG_KEYS, aggKeys);
+ Map<String, String> properties = new HashMap<String, String>() {
+ {
+ put("replication_num", String.valueOf(Math.max(1,
+ Config.min_replication_num_per_tablet)));
+ }
+ };
+ TableName tableName = new TableName("internal", "demo",
"testToSqlWithComment1");
+ CreateTableStmt createTableStmt = new CreateTableStmt(true, false,
+ tableName, columnDefs, engineName, keysDesc, null, null,
+ properties, null, "xxx", null);
+ String createTableSql = "CREATE TABLE IF NOT EXISTS
`demo`.`testToSqlWithComment1` (\n"
+ + " `a` BIGINT NOT NULL COMMENT \"\",\n"
+ + " `b` INT NOT NULL COMMENT \"\"\n"
+ + ") ENGINE = olap\n"
+ + "AGGREGATE KEY(`a`)\n"
+ + "COMMENT \"xxx\"\n"
+ + "PROPERTIES (\"replication_num\" = \"1\")";
+ Assert.assertEquals(createTableStmt.toSql(), createTableSql);
+
+
+ columnDefs.add(new ColumnDef("c",
TypeDef.create(PrimitiveType.STRING), true));
+ columnDefs.add(new ColumnDef("d",
TypeDef.create(PrimitiveType.DOUBLE), true));
+ columnDefs.add(new ColumnDef("e",
TypeDef.create(PrimitiveType.DECIMAL128), false));
+ columnDefs.add(new ColumnDef("f", TypeDef.create(PrimitiveType.DATE),
false));
+ columnDefs.add(new ColumnDef("g",
TypeDef.create(PrimitiveType.SMALLINT), false));
+ columnDefs.add(new ColumnDef("h",
TypeDef.create(PrimitiveType.BOOLEAN), false));
+ aggKeys = Lists.newArrayList("a", "d", "f");
+ keysDesc = new KeysDesc(KeysType.DUP_KEYS, aggKeys);
+ properties = new HashMap<String, String>() {
+ {
+ put("replication_num", String.valueOf(Math.max(10,
+ Config.min_replication_num_per_tablet)));
+ }
+ };
+ tableName = new TableName("internal", "demo", "testToSqlWithComment2");
+ createTableStmt = new CreateTableStmt(false, false,
+ tableName, columnDefs, engineName, keysDesc, null, null,
+ properties, null, "xxx", null);
+ createTableSql = "CREATE TABLE `demo`.`testToSqlWithComment2` (\n"
+ + " `a` BIGINT NOT NULL COMMENT \"\",\n"
+ + " `b` INT NOT NULL COMMENT \"\",\n"
+ + " `c` TEXT NULL COMMENT \"\",\n"
+ + " `d` DOUBLE NULL COMMENT \"\",\n"
+ + " `e` DECIMALV3(38, 0) NOT NULL COMMENT \"\",\n"
+ + " `f` DATE NOT NULL COMMENT \"\",\n"
+ + " `g` SMALLINT NOT NULL COMMENT \"\",\n"
+ + " `h` BOOLEAN NOT NULL COMMENT \"\"\n"
+ + ") ENGINE = olap\n"
+ + "DUPLICATE KEY(`a`, `d`, `f`)\n"
+ + "COMMENT \"xxx\"\n"
+ + "PROPERTIES (\"replication_num\" = \"10\")";
+ Assert.assertEquals(createTableStmt.toSql(), createTableSql);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
