Skip to content

Commit

Permalink
[fix](stmt):fix CreateTableStmt toSql placed comment in wrong place (a…
Browse files Browse the repository at this point in the history 
…pache#27504)

Issue Number: close apache#27474
Co-authored-by: tongyang.han <tongyang.han@jiduauto.com>
  • Loading branch information
@htyoung @HappenLee
htyoung authored and HappenLee committed Jan 12, 2024
1 parent 6c9c3c2 commit 5d9dda0
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 98 deletions.
159 changes: 61 additions & 98 deletions fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -130,48 +129,26 @@ public CreateTableStmt() {
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();
Expand All @@ -198,20 +175,10 @@ public CreateTableStmt(boolean ifNotExists,
}

// 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;
Expand Down Expand Up @@ -308,8 +275,8 @@ public List<Index> getIndexes() {
}

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

Expand All @@ -319,19 +286,19 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
// 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");
}

analyzeEngineName();

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.");
Expand All @@ -340,14 +307,14 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
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();
Expand All @@ -361,9 +328,9 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
}
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());
}
}
Expand All @@ -374,8 +341,8 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
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;
Expand Down Expand Up @@ -411,7 +378,7 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
} 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.");
}
}

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

// 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));
Expand Down Expand Up @@ -502,14 +467,14 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
}
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()
Expand All @@ -526,17 +491,17 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
}
}

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

}
Expand All @@ -553,10 +518,10 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
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");
}
}
}
Expand All @@ -565,8 +530,8 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
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");
}
}

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

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()) {
Expand All @@ -599,13 +564,11 @@ public void analyze(Analyzer analyzer) throws UserException, AnalysisException {
}
}
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())));
Expand Down Expand Up @@ -687,12 +650,16 @@ public String toSql() {
}
}
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());
}
Expand All @@ -701,7 +668,7 @@ public String toSql() {
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++) {
Expand All @@ -719,20 +686,16 @@ public String toSql() {
// 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();
}

Expand Down
Loading

0 comments on commit 5d9dda0

Please sign in to comment.