From 06d79998a7f96e63fcefa547113cdf7a168ef381 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 08:03:01 +0000 Subject: [PATCH] [BugFix] When the reserved keyword uses backquotes as input, deserialization causes parseSqlToExpr syntax error, which results in log edit failure. (backport #51677) (#51878) Co-authored-by: zhanghe --- .../java/com/starrocks/analysis/SlotRef.java | 18 ++- .../java/com/starrocks/catalog/ColumnId.java | 9 ++ .../catalog/ExpressionRangePartitionInfo.java | 6 + .../com/starrocks/persist/ColumnIdExpr.java | 5 +- .../com/starrocks/sql/ast/Identifier.java | 10 ++ .../com/starrocks/sql/parser/AstBuilder.java | 10 +- .../ExpressionRangePartitionInfoTest.java | 108 ++++++++++++++++++ .../R/test_automatic_partition | 4 +- 8 files changed, 162 insertions(+), 8 deletions(-) diff --git a/fe/fe-core/src/main/java/com/starrocks/analysis/SlotRef.java b/fe/fe-core/src/main/java/com/starrocks/analysis/SlotRef.java index c79ec619939b3..75818f784cf51 100644 --- a/fe/fe-core/src/main/java/com/starrocks/analysis/SlotRef.java +++ b/fe/fe-core/src/main/java/com/starrocks/analysis/SlotRef.java @@ -65,8 +65,9 @@ public class SlotRef extends Expr { private TableName tblName; private String colName; private ColumnId columnId; - // Used in toSql + //label/isBackQuoted used in toSql private String label; + private boolean isBackQuoted = false; private QualifiedName qualifiedName; @@ -172,6 +173,14 @@ public SlotRef(SlotId slotId) { this(new SlotDescriptor(slotId, "", Type.INVALID, false)); } + public void setBackQuoted(boolean isBackQuoted) { + this.isBackQuoted = isBackQuoted; + } + + public boolean isBackQuoted() { + return isBackQuoted; + } + public QualifiedName getQualifiedName() { return qualifiedName; } @@ -283,7 +292,12 @@ public String toSqlImpl() { if (tblName != null && !isFromLambda()) { return tblName.toSql() + "." + "`" + colName + "`"; } else if (label != null) { - return label; + if (isBackQuoted && !(label.startsWith("`") && label.endsWith("`"))) { + sb.append("`").append(label).append("`"); + return sb.toString(); + } else { + return label; + } } else if (desc.getSourceExprs() != null) { sb.append(""); for (Expr expr : desc.getSourceExprs()) { diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/ColumnId.java b/fe/fe-core/src/main/java/com/starrocks/catalog/ColumnId.java index 579e941a70c0f..d43e43a5fe26f 100644 --- a/fe/fe-core/src/main/java/com/starrocks/catalog/ColumnId.java +++ b/fe/fe-core/src/main/java/com/starrocks/catalog/ColumnId.java @@ -15,6 +15,8 @@ package com.starrocks.catalog; +import com.starrocks.common.util.ParseUtil; + import java.util.Comparator; import java.util.Objects; @@ -73,6 +75,13 @@ public String toString() { return getId(); } + public String toSql(boolean isBackQuoted) { + if (isBackQuoted) { + return ParseUtil.backquote(id); + } + return id; + } + public static final Comparator CASE_INSENSITIVE_ORDER = new ColumnId.CaseInsensitiveComparator(); diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/ExpressionRangePartitionInfo.java b/fe/fe-core/src/main/java/com/starrocks/catalog/ExpressionRangePartitionInfo.java index 329602d171ad6..3505b2168dbb0 100644 --- a/fe/fe-core/src/main/java/com/starrocks/catalog/ExpressionRangePartitionInfo.java +++ b/fe/fe-core/src/main/java/com/starrocks/catalog/ExpressionRangePartitionInfo.java @@ -14,6 +14,7 @@ package com.starrocks.catalog; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -169,6 +170,11 @@ public ExpressionRangePartitionInfo(List partitionExprs, List getPartitionExprs() { + return partitionExprs; + } + public List getPartitionExprs(Map idToColumn) { List result = new ArrayList<>(partitionExprs.size()); for (ColumnIdExpr columnIdExpr : partitionExprs) { diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/ColumnIdExpr.java b/fe/fe-core/src/main/java/com/starrocks/persist/ColumnIdExpr.java index 414fc4f6948f4..d50df3b194bb9 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/ColumnIdExpr.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/ColumnIdExpr.java @@ -18,6 +18,7 @@ import com.starrocks.analysis.SlotRef; import com.starrocks.catalog.Column; import com.starrocks.catalog.ColumnId; +import com.starrocks.common.util.ParseUtil; import com.starrocks.qe.SqlModeHelper; import com.starrocks.sql.analyzer.AstToStringBuilder; import com.starrocks.sql.analyzer.SemanticException; @@ -135,9 +136,9 @@ private static class ExprSerializeVisitor extends AstToStringBuilder.AST2StringB @Override public String visitSlot(SlotRef node, Void context) { if (node.getTblNameWithoutAnalyzed() != null) { - return node.getTblNameWithoutAnalyzed().toString() + "." + node.getColumnId().getId(); + return node.getTblNameWithoutAnalyzed().toSql() + "." + ParseUtil.backquote(node.getColumnId().getId()); } else { - return node.getColumnId().getId(); + return node.getColumnId().toSql(node.isBackQuoted()); } } } diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/ast/Identifier.java b/fe/fe-core/src/main/java/com/starrocks/sql/ast/Identifier.java index ed221fbd50819..2c821ec60521d 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/ast/Identifier.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/ast/Identifier.java @@ -22,6 +22,8 @@ public class Identifier implements ParseNode { private final NodePosition pos; + private boolean isBackQuoted = false; + public Identifier(String value) { this(value, NodePosition.ZERO); } @@ -40,6 +42,14 @@ public NodePosition getPos() { return pos; } + public void setBackQuoted(boolean isBackQuoted) { + this.isBackQuoted = isBackQuoted; + } + + public boolean isBackQuoted() { + return isBackQuoted; + } + @Override public R accept(AstVisitor visitor, C context) { return null; diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java index bf07bc4274048..f1cc6f18fb31c 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java @@ -6822,7 +6822,11 @@ public ParseNode visitColumnReference(StarRocksParser.ColumnReferenceContext con List parts = new ArrayList<>(); parts.add(identifier.getValue()); QualifiedName qualifiedName = QualifiedName.of(parts, createPos(context)); - return new SlotRef(qualifiedName); + SlotRef slotRef = new SlotRef(qualifiedName); + if (identifier.isBackQuoted()) { + slotRef.setBackQuoted(true); + } + return slotRef; } @Override @@ -6901,7 +6905,9 @@ public ParseNode visitUnquotedIdentifier(StarRocksParser.UnquotedIdentifierConte @Override public ParseNode visitBackQuotedIdentifier(StarRocksParser.BackQuotedIdentifierContext context) { - return new Identifier(context.getText().replace("`", ""), createPos(context)); + Identifier backQuotedIdentifier = new Identifier(context.getText().replace("`", ""), createPos(context)); + backQuotedIdentifier.setBackQuoted(true); + return backQuotedIdentifier; } @Override diff --git a/fe/fe-core/src/test/java/com/starrocks/catalog/ExpressionRangePartitionInfoTest.java b/fe/fe-core/src/test/java/com/starrocks/catalog/ExpressionRangePartitionInfoTest.java index d630940b6c9ac..7c550e4bc2e11 100644 --- a/fe/fe-core/src/test/java/com/starrocks/catalog/ExpressionRangePartitionInfoTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/catalog/ExpressionRangePartitionInfoTest.java @@ -28,6 +28,7 @@ import com.starrocks.qe.ConnectContext; import com.starrocks.server.GlobalStateMgr; import com.starrocks.sql.ast.CreateTableStmt; +import com.starrocks.sql.ast.ExpressionPartitionDesc; import com.starrocks.sql.ast.PartitionKeyDesc; import com.starrocks.sql.ast.PartitionKeyDesc.PartitionRangeType; import com.starrocks.sql.ast.PartitionValue; @@ -592,4 +593,111 @@ public void testExpressionRangePartitionInfoV2SerializedWrongNotFailed() throws OlapTable readTable = GsonUtils.GSON.fromJson(json, OlapTable.class); } + @Test + public void testExpressionRangePartitionInfoWithReservedKeywordSerialized() throws Exception { + ConnectContext ctx = starRocksAssert.getCtx(); + String createSQL = "CREATE TABLE table_reserverd_keyword_partition (\n" + + "databaseName varchar(200) NULL COMMENT \"\",\n" + + "tableName varchar(200) NULL COMMENT \"\",\n" + + "queryTime varchar(50) NULL COMMENT \"\",\n" + + "queryId varchar(50) NULL COMMENT \"\",\n" + + "partitionHitSum int(11) NULL COMMENT \"\",\n" + + "partitionSum int(11) NULL COMMENT \"\",\n" + + "tabletHitNum int(11) NULL COMMENT \"\",\n" + + "tabletSum int(11) NULL COMMENT \"\",\n" + + "startHitPartition varchar(20) NULL COMMENT \"\",\n" + + "`partition` date NULL COMMENT \"\",\n" + + "clusterAddress varchar(50) NULL COMMENT \"\",\n" + + "costTime int(11) NULL COMMENT \"\",\n" + + "tableQueryCount int(11) NULL COMMENT \"\"\n" + + ") ENGINE=OLAP\n" + + "DUPLICATE KEY(databaseName, tableName)\n" + + "COMMENT \"OLAP\"\n" + + "PARTITION BY date_trunc('day', `partition`)\n" + + "DISTRIBUTED BY HASH(databaseName) BUCKETS 1\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"in_memory\" = \"false\",\n" + + "\"enable_persistent_index\" = \"false\",\n" + + "\"replicated_storage\" = \"true\",\n" + + "\"compression\" = \"LZ4\"\n" + + ");"; + CreateTableStmt createTableStmt = (CreateTableStmt) UtFrameUtils.parseStmtWithNewParser(createSQL, ctx); + ExpressionPartitionDesc exprPartitiondesc = (ExpressionPartitionDesc) createTableStmt.getPartitionDesc(); + FunctionCallExpr funExpr = (FunctionCallExpr) exprPartitiondesc.getExpr(); + SlotRef slotRef = (SlotRef) funExpr.getChild(1); + Assert.assertTrue(slotRef.isBackQuoted()); + + + StarRocksAssert.utCreateTableWithRetry(createTableStmt); + Database db = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb("test"); + Table table = db.getTable("table_reserverd_keyword_partition"); + ExpressionRangePartitionInfo expressionRangePartitionInfo = + (ExpressionRangePartitionInfo) ((OlapTable) table).getPartitionInfo(); + String exprToSql = expressionRangePartitionInfo.getPartitionExprs().get(0).toSql(); + Assert.assertEquals("date_trunc('day', `partition`)", exprToSql); + // serialize + String json = GsonUtils.GSON.toJson(table); + // deserialize + OlapTable readTable = GsonUtils.GSON.fromJson(json, OlapTable.class); + expressionRangePartitionInfo = (ExpressionRangePartitionInfo) readTable.getPartitionInfo(); + List readPartitionExprs = expressionRangePartitionInfo.getPartitionExprs(); + Function fn = readPartitionExprs.get(0).getExpr().getFn(); + Assert.assertNotNull(fn); + } + + @Test + public void testExpressionRangePartitionInfoWithReservedKeywordSerializedWrong() throws Exception { + ConnectContext ctx = starRocksAssert.getCtx(); + String createSQL = "CREATE TABLE table_reserverd_keyword_partition1 (\n" + + "databaseName varchar(200) NULL COMMENT \"\",\n" + + "tableName varchar(200) NULL COMMENT \"\",\n" + + "queryTime varchar(50) NULL COMMENT \"\",\n" + + "queryId varchar(50) NULL COMMENT \"\",\n" + + "partitionHitSum int(11) NULL COMMENT \"\",\n" + + "partitionSum int(11) NULL COMMENT \"\",\n" + + "tabletHitNum int(11) NULL COMMENT \"\",\n" + + "tabletSum int(11) NULL COMMENT \"\",\n" + + "startHitPartition varchar(20) NULL COMMENT \"\",\n" + + "`partition` date NULL COMMENT \"\",\n" + + "clusterAddress varchar(50) NULL COMMENT \"\",\n" + + "costTime int(11) NULL COMMENT \"\",\n" + + "tableQueryCount int(11) NULL COMMENT \"\"\n" + + ") ENGINE=OLAP\n" + + "DUPLICATE KEY(databaseName, tableName)\n" + + "COMMENT \"OLAP\"\n" + + "PARTITION BY date_trunc('day', `partition`)\n" + + "DISTRIBUTED BY HASH(databaseName) BUCKETS 1\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"in_memory\" = \"false\",\n" + + "\"enable_persistent_index\" = \"false\",\n" + + "\"replicated_storage\" = \"true\",\n" + + "\"compression\" = \"LZ4\"\n" + + ");"; + CreateTableStmt createTableStmt = (CreateTableStmt) UtFrameUtils.parseStmtWithNewParser(createSQL, ctx); + ExpressionPartitionDesc exprPartitiondesc = (ExpressionPartitionDesc) createTableStmt.getPartitionDesc(); + FunctionCallExpr funExpr = (FunctionCallExpr) exprPartitiondesc.getExpr(); + SlotRef slotRef = (SlotRef) funExpr.getChild(1); + Assert.assertTrue(slotRef.isBackQuoted()); + slotRef.setBackQuoted(false); + + try { + // serialize in OnCrete Function will throw err,because of "date_trunc('day', partition)". + GlobalStateMgr.getCurrentState().getLocalMetastore().createTable(createTableStmt); + Assert.fail(); + } catch (Exception e) { + e.printStackTrace(); + Assert.assertTrue(e.getMessage().contains("Getting syntax error at line 1, column 10. " + + "Detail message: Unexpected input '(', the most similar input is {}.")); + } + + //the table still create successfully. + Database db = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb("test"); + Table table = db.getTable("table_reserverd_keyword_partition1"); + ExpressionRangePartitionInfo expressionRangePartitionInfo = + (ExpressionRangePartitionInfo) ((OlapTable) table).getPartitionInfo(); + String exprToSql = expressionRangePartitionInfo.getPartitionExprs().get(0).toSql(); + Assert.assertEquals("date_trunc('day', partition)", exprToSql); + } } diff --git a/test/sql/test_automatic_bucket/R/test_automatic_partition b/test/sql/test_automatic_bucket/R/test_automatic_partition index 9edf9e2f57d2b..472ca5993b535 100644 --- a/test/sql/test_automatic_bucket/R/test_automatic_partition +++ b/test/sql/test_automatic_bucket/R/test_automatic_partition @@ -508,7 +508,7 @@ t CREATE TABLE `t` ( `v` int(11) NULL COMMENT "" ) ENGINE=OLAP DUPLICATE KEY(`k`, `v`) -PARTITION BY date_trunc('DAY', k) +PARTITION BY date_trunc('DAY', `k`) DISTRIBUTED BY RANDOM PROPERTIES ( "bucket_size" = "1", @@ -540,7 +540,7 @@ t CREATE TABLE `t` ( `v` int(11) NULL COMMENT "" ) ENGINE=OLAP DUPLICATE KEY(`k`, `v`) -PARTITION BY date_trunc('DAY', k) +PARTITION BY date_trunc('DAY', `k`) DISTRIBUTED BY RANDOM PROPERTIES ( "bucket_size" = "1",