Skip to content

Commit

Permalink
[BugFix] When the reserved keyword uses backquotes as input, deserial…
Browse files Browse the repository at this point in the history 
…ization causes parseSqlToExpr syntax error, which results in log edit failure. (backport #51677) (#51878)

Co-authored-by: zhanghe <[email protected]>
  • Loading branch information
@mergify @zhangheihei
mergify[bot] and zhangheihei authored Oct 14, 2024
1 parent e2983e0 commit 06d7999
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 8 deletions.
18 changes: 16 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/analysis/SlotRef.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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("<slot ").append(desc.getId().asInt()).append(">");
for (Expr expr : desc.getSourceExprs()) {
Expand Down
9 changes: 9 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/catalog/ColumnId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package com.starrocks.catalog;


import com.starrocks.common.util.ParseUtil;

import java.util.Comparator;
import java.util.Objects;

Expand Down Expand Up @@ -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<ColumnId> CASE_INSENSITIVE_ORDER =
new ColumnId.CaseInsensitiveComparator();

Expand Down
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/catalog/ExpressionRangePartitionInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -169,6 +170,11 @@ public ExpressionRangePartitionInfo(List<ColumnIdExpr> partitionExprs, List<Colu
this.type = type;
}

@VisibleForTesting
public List<ColumnIdExpr> getPartitionExprs() {
return partitionExprs;
}

public List<Expr> getPartitionExprs(Map<ColumnId, Column> idToColumn) {
List<Expr> result = new ArrayList<>(partitionExprs.size());
for (ColumnIdExpr columnIdExpr : partitionExprs) {
Expand Down
5 changes: 3 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/persist/ColumnIdExpr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/sql/ast/Identifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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, C> R accept(AstVisitor<R, C> visitor, C context) {
return null;
Expand Down
10 changes: 8 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6822,7 +6822,11 @@ public ParseNode visitColumnReference(StarRocksParser.ColumnReferenceContext con
List<String> 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
Expand Down Expand Up @@ -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
Expand Down
108 changes: 108 additions & 0 deletions fe/fe-core/src/test/java/com/starrocks/catalog/ExpressionRangePartitionInfoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ColumnIdExpr> 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 {<EOF>}."));
}

//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);
}
}
4 changes: 2 additions & 2 deletions test/sql/test_automatic_bucket/R/test_automatic_partition
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 06d7999

Please sign in to comment.