Skip to content

Commit ca3426e

Browse files
author
tongyang.han
committed
[fix](stmt):fix CreateTableStmt toSql placed comment in wrong place
1 parent d2275e7 commit ca3426e

File tree

2 files changed

+123
-98
lines changed

2 files changed

+123
-98
lines changed

‎fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java

+61-98
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ public class CreateTableStmt extends DdlStmt {
8585

8686
// set in analyze
8787
private List<Column> columns = Lists.newArrayList();
88-
8988
private List<Index> indexes = Lists.newArrayList();
9089

9190
static {
@@ -130,48 +129,26 @@ public CreateTableStmt() {
130129
columnDefs = Lists.newArrayList();
131130
}
132131

133-
public CreateTableStmt(boolean ifNotExists,
134-
boolean isExternal,
135-
TableName tableName,
136-
List<ColumnDef> columnDefinitions,
137-
String engineName,
138-
KeysDesc keysDesc,
139-
PartitionDesc partitionDesc,
140-
DistributionDesc distributionDesc,
141-
Map<String, String> properties,
142-
Map<String, String> extProperties,
132+
public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName tableName,
133+
List<ColumnDef> columnDefinitions, String engineName, KeysDesc keysDesc, PartitionDesc partitionDesc,
134+
DistributionDesc distributionDesc, Map<String, String> properties, Map<String, String> extProperties,
143135
String comment) {
144136
this(ifNotExists, isExternal, tableName, columnDefinitions, null, engineName, keysDesc, partitionDesc,
145137
distributionDesc, properties, extProperties, comment, null);
146138
}
147139

148-
public CreateTableStmt(boolean ifNotExists,
149-
boolean isExternal,
150-
TableName tableName,
151-
List<ColumnDef> columnDefinitions,
152-
String engineName,
153-
KeysDesc keysDesc,
154-
PartitionDesc partitionDesc,
155-
DistributionDesc distributionDesc,
156-
Map<String, String> properties,
157-
Map<String, String> extProperties,
140+
public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName tableName,
141+
List<ColumnDef> columnDefinitions, String engineName, KeysDesc keysDesc, PartitionDesc partitionDesc,
142+
DistributionDesc distributionDesc, Map<String, String> properties, Map<String, String> extProperties,
158143
String comment, List<AlterClause> ops) {
159144
this(ifNotExists, isExternal, tableName, columnDefinitions, null, engineName, keysDesc, partitionDesc,
160145
distributionDesc, properties, extProperties, comment, ops);
161146
}
162147

163-
public CreateTableStmt(boolean ifNotExists,
164-
boolean isExternal,
165-
TableName tableName,
166-
List<ColumnDef> columnDefinitions,
167-
List<IndexDef> indexDefs,
168-
String engineName,
169-
KeysDesc keysDesc,
170-
PartitionDesc partitionDesc,
171-
DistributionDesc distributionDesc,
172-
Map<String, String> properties,
173-
Map<String, String> extProperties,
174-
String comment, List<AlterClause> rollupAlterClauseList) {
148+
public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName tableName,
149+
List<ColumnDef> columnDefinitions, List<IndexDef> indexDefs, String engineName, KeysDesc keysDesc,
150+
PartitionDesc partitionDesc, DistributionDesc distributionDesc, Map<String, String> properties,
151+
Map<String, String> extProperties, String comment, List<AlterClause> rollupAlterClauseList) {
175152
this.tableName = tableName;
176153
if (columnDefinitions == null) {
177154
this.columnDefs = Lists.newArrayList();
@@ -198,20 +175,10 @@ public CreateTableStmt(boolean ifNotExists,
198175
}
199176

200177
// for Nereids
201-
public CreateTableStmt(boolean ifNotExists,
202-
boolean isExternal,
203-
TableName tableName,
204-
List<Column> columns,
205-
List<Index> indexes,
206-
String engineName,
207-
KeysDesc keysDesc,
208-
PartitionDesc partitionDesc,
209-
DistributionDesc distributionDesc,
210-
Map<String, String> properties,
211-
Map<String, String> extProperties,
212-
String comment,
213-
List<AlterClause> rollupAlterClauseList,
214-
Void unused) {
178+
public CreateTableStmt(boolean ifNotExists, boolean isExternal, TableName tableName, List<Column> columns,
179+
List<Index> indexes, String engineName, KeysDesc keysDesc, PartitionDesc partitionDesc,
180+
DistributionDesc distributionDesc, Map<String, String> properties, Map<String, String> extProperties,
181+
String comment, List<AlterClause> rollupAlterClauseList, Void unused) {
215182
this.ifNotExists = ifNotExists;
216183
this.isExternal = isExternal;
217184
this.tableName = tableName;
@@ -308,8 +275,8 @@ public List<Index> getIndexes() {
308275
}
309276

310277
@Override
311-
public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
312-
if (Strings.isNullOrEmpty(engineName) || engineName.equalsIgnoreCase("olap")) {
278+
public void analyze(Analyzer analyzer) throws UserException {
279+
if (Strings.isNullOrEmpty(engineName) || engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
313280
this.properties = maybeRewriteByAutoBucket(distributionDesc, properties);
314281
}
315282

@@ -319,19 +286,19 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
319286
// disallow external catalog
320287
Util.prohibitExternalCatalog(tableName.getCtl(), this.getClass().getSimpleName());
321288

322-
if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ConnectContext.get(), tableName.getDb(),
323-
tableName.getTbl(), PrivPredicate.CREATE)) {
289+
if (!Env.getCurrentEnv().getAccessManager()
290+
.checkTblPriv(ConnectContext.get(), tableName.getDb(), tableName.getTbl(), PrivPredicate.CREATE)) {
324291
ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "CREATE");
325292
}
326293

327294
analyzeEngineName();
328295

329296
boolean enableDuplicateWithoutKeysByDefault = false;
330297
if (properties != null) {
331-
enableDuplicateWithoutKeysByDefault =
332-
PropertyAnalyzer.analyzeEnableDuplicateWithoutKeysByDefault(properties);
298+
enableDuplicateWithoutKeysByDefault = PropertyAnalyzer.analyzeEnableDuplicateWithoutKeysByDefault(
299+
properties);
333300
}
334-
//pre-block creation with column type ALL
301+
// pre-block creation with column type ALL
335302
for (ColumnDef columnDef : columnDefs) {
336303
if (Objects.equals(columnDef.getType(), Type.ALL)) {
337304
throw new AnalysisException("Disable to create table with `ALL` type columns.");
@@ -340,14 +307,14 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
340307
throw new AnalysisException("Disable to create table with `DATE` type columns, please use `DATEV2`.");
341308
}
342309
if (Objects.equals(columnDef.getType(), Type.DECIMALV2) && Config.disable_decimalv2) {
343-
throw new AnalysisException("Disable to create table with `DECIMAL` type columns,"
344-
+ "please use `DECIMALV3`.");
310+
throw new AnalysisException(
311+
"Disable to create table with `DECIMAL` type columns," + "please use `DECIMALV3`.");
345312
}
346313
}
347314

348315
boolean enableUniqueKeyMergeOnWrite = false;
349316
// analyze key desc
350-
if (engineName.equalsIgnoreCase("olap")) {
317+
if (engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
351318
// olap table
352319
if (keysDesc == null) {
353320
List<String> keysColumnNames = Lists.newArrayList();
@@ -361,9 +328,9 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
361328
}
362329
if (hasAggregate) {
363330
for (ColumnDef columnDef : columnDefs) {
364-
if (columnDef.getAggregateType() == null
365-
&& !columnDef.getType().isScalarType(PrimitiveType.STRING)
366-
&& !columnDef.getType().isScalarType(PrimitiveType.JSONB)) {
331+
if (columnDef.getAggregateType() == null && !columnDef.getType()
332+
.isScalarType(PrimitiveType.STRING) && !columnDef.getType()
333+
.isScalarType(PrimitiveType.JSONB)) {
367334
keysColumnNames.add(columnDef.getName());
368335
}
369336
}
@@ -374,8 +341,8 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
374341
keyLength += columnDef.getType().getIndexSize();
375342
if (keysColumnNames.size() >= FeConstants.shortkey_max_column_count
376343
|| keyLength > FeConstants.shortkey_maxsize_bytes) {
377-
if (keysColumnNames.size() == 0
378-
&& columnDef.getType().getPrimitiveType().isCharFamily()) {
344+
if (keysColumnNames.isEmpty() && columnDef.getType().getPrimitiveType()
345+
.isCharFamily()) {
379346
keysColumnNames.add(columnDef.getName());
380347
}
381348
break;
@@ -411,7 +378,7 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
411378
} else {
412379
if (enableDuplicateWithoutKeysByDefault) {
413380
throw new AnalysisException("table property 'enable_duplicate_without_keys_by_default' only can"
414-
+ " set 'true' when create olap table by default.");
381+
+ " set 'true' when create olap table by default.");
415382
}
416383
}
417384

@@ -463,13 +430,11 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
463430
}
464431

465432
// analyze column def
466-
if (!(engineName.equals("elasticsearch"))
467-
&& (columnDefs == null || columnDefs.isEmpty())) {
433+
if (!(engineName.equalsIgnoreCase("elasticsearch")) && (columnDefs == null || columnDefs.isEmpty())) {
468434
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLE_MUST_HAVE_COLUMNS);
469435
}
470436
// add a hidden column as delete flag for unique table
471-
if (Config.enable_batch_delete_by_default
472-
&& keysDesc != null
437+
if (Config.enable_batch_delete_by_default && keysDesc != null
473438
&& keysDesc.getKeysType() == KeysType.UNIQUE_KEYS) {
474439
if (enableUniqueKeyMergeOnWrite) {
475440
columnDefs.add(ColumnDef.newDeleteSignColumnDef(AggregateType.NONE));
@@ -502,14 +467,14 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
502467
}
503468
Set<String> columnSet = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
504469
for (ColumnDef columnDef : columnDefs) {
505-
columnDef.analyze(engineName.equals("olap"));
470+
columnDef.analyze(engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME));
506471

507-
if (columnDef.getType().isComplexType() && engineName.equals("olap")) {
508-
if (columnDef.getAggregateType() != null
509-
&& columnDef.getAggregateType() != AggregateType.NONE
472+
if (columnDef.getType().isComplexType() && engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
473+
if (columnDef.getAggregateType() != null && columnDef.getAggregateType() != AggregateType.NONE
510474
&& columnDef.getAggregateType() != AggregateType.REPLACE) {
511-
throw new AnalysisException(columnDef.getType().getPrimitiveType()
512-
+ " column can't support aggregation " + columnDef.getAggregateType());
475+
throw new AnalysisException(
476+
columnDef.getType().getPrimitiveType() + " column can't support aggregation "
477+
+ columnDef.getAggregateType());
513478
}
514479
if (columnDef.isKey()) {
515480
throw new AnalysisException(columnDef.getType().getPrimitiveType()
@@ -526,17 +491,17 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
526491
}
527492
}
528493

529-
if (engineName.equals("olap")) {
494+
if (engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
530495
// before analyzing partition, handle the replication allocation info
531-
properties = PropertyAnalyzer.rewriteReplicaAllocationProperties(
532-
tableName.getCtl(), tableName.getDb(), properties);
496+
properties = PropertyAnalyzer.rewriteReplicaAllocationProperties(tableName.getCtl(), tableName.getDb(),
497+
properties);
533498
// analyze partition
534499
if (partitionDesc != null) {
535500
if (partitionDesc instanceof ListPartitionDesc || partitionDesc instanceof RangePartitionDesc) {
536501
partitionDesc.analyze(columnDefs, properties);
537502
} else {
538-
throw new AnalysisException("Currently only support range"
539-
+ " and list partition with engine type olap");
503+
throw new AnalysisException(
504+
"Currently only support range" + " and list partition with engine type olap");
540505
}
541506

542507
}
@@ -553,10 +518,10 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
553518
for (ColumnDef columnDef : columnDefs) {
554519
if (columnDef.getAggregateType() == AggregateType.REPLACE
555520
|| columnDef.getAggregateType() == AggregateType.REPLACE_IF_NOT_NULL) {
556-
throw new AnalysisException("Create aggregate keys table with value columns of which"
557-
+ " aggregate type is " + columnDef.getAggregateType()
558-
+ " should not contain random"
559-
+ " distribution desc");
521+
throw new AnalysisException(
522+
"Create aggregate keys table with value columns of which" + " aggregate type is "
523+
+ columnDef.getAggregateType() + " should not contain random"
524+
+ " distribution desc");
560525
}
561526
}
562527
}
@@ -565,8 +530,8 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
565530
EsUtil.analyzePartitionAndDistributionDesc(partitionDesc, distributionDesc);
566531
} else {
567532
if (partitionDesc != null || distributionDesc != null) {
568-
throw new AnalysisException("Create " + engineName
569-
+ " table should not contain partition or distribution desc");
533+
throw new AnalysisException(
534+
"Create " + engineName + " table should not contain partition or distribution desc");
570535
}
571536
}
572537

@@ -586,7 +551,7 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
586551

587552
for (IndexDef indexDef : indexDefs) {
588553
indexDef.analyze();
589-
if (!engineName.equalsIgnoreCase("olap")) {
554+
if (!engineName.equalsIgnoreCase(DEFAULT_ENGINE_NAME)) {
590555
throw new AnalysisException("index only support in olap engine at current version.");
591556
}
592557
for (String indexColName : indexDef.getColumns()) {
@@ -599,13 +564,11 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
599564
}
600565
}
601566
if (!found) {
602-
throw new AnalysisException("Column does not exist in table. invalid column: "
603-
+ indexColName);
567+
throw new AnalysisException("Column does not exist in table. invalid column: " + indexColName);
604568
}
605569
}
606-
indexes.add(new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(),
607-
indexDef.getColumns(), indexDef.getIndexType(),
608-
indexDef.getProperties(), indexDef.getComment()));
570+
indexes.add(new Index(Env.getCurrentEnv().getNextId(), indexDef.getIndexName(), indexDef.getColumns(),
571+
indexDef.getIndexType(), indexDef.getProperties(), indexDef.getComment()));
609572
distinct.add(indexDef.getIndexName());
610573
distinctCol.add(Pair.of(indexDef.getIndexType(),
611574
indexDef.getColumns().stream().map(String::toUpperCase).collect(Collectors.toList())));
@@ -687,12 +650,16 @@ public String toSql() {
687650
}
688651
}
689652
sb.append("\n)");
690-
sb.append(" ENGINE = ").append(engineName);
653+
sb.append(" ENGINE = ").append(engineName.toLowerCase());
691654

692655
if (keysDesc != null) {
693656
sb.append("\n").append(keysDesc.toSql());
694657
}
695658

659+
if (!Strings.isNullOrEmpty(comment)) {
660+
sb.append("\nCOMMENT \"").append(comment).append("\"");
661+
}
662+
696663
if (partitionDesc != null) {
697664
sb.append("\n").append(partitionDesc.toSql());
698665
}
@@ -701,7 +668,7 @@ public String toSql() {
701668
sb.append("\n").append(distributionDesc.toSql());
702669
}
703670

704-
if (rollupAlterClauseList != null && rollupAlterClauseList.size() != 0) {
671+
if (rollupAlterClauseList != null && !rollupAlterClauseList.isEmpty()) {
705672
sb.append("\n rollup(");
706673
StringBuilder opsSb = new StringBuilder();
707674
for (int i = 0; i < rollupAlterClauseList.size(); i++) {
@@ -719,20 +686,16 @@ public String toSql() {
719686
// which is implemented in Catalog.getDdlStmt()
720687
if (properties != null && !properties.isEmpty()) {
721688
sb.append("\nPROPERTIES (");
722-
sb.append(new PrintableMap<String, String>(properties, " = ", true, true, true));
689+
sb.append(new PrintableMap<>(properties, " = ", true, true, true));
723690
sb.append(")");
724691
}
725692

726693
if (extProperties != null && !extProperties.isEmpty()) {
727694
sb.append("\n").append(engineName.toUpperCase()).append(" PROPERTIES (");
728-
sb.append(new PrintableMap<String, String>(extProperties, " = ", true, true, true));
695+
sb.append(new PrintableMap<>(extProperties, " = ", true, true, true));
729696
sb.append(")");
730697
}
731698

732-
if (!Strings.isNullOrEmpty(comment)) {
733-
sb.append("\nCOMMENT \"").append(comment).append("\"");
734-
}
735-
736699
return sb.toString();
737700
}
738701

0 commit comments

Comments
 (0)