From ca4e9037e077cdb3b42ddc5d18131bf651b10fb8 Mon Sep 17 00:00:00 2001 From: Binglin Chang Date: Thu, 24 Nov 2022 21:19:18 +0800 Subject: [PATCH] [Feature] delete syntax support using clause (#13757) This PR supports using clause in delete syntax, so user can delete rows specified by join with other tables. Like in https://www.postgresql.org/docs/current/sql-delete.html DELETE FROM films USING producers WHERE producer_id = producers.id AND producers.name = 'foo'; --- .../com/starrocks/load/DeleteHandler.java | 2 +- .../java/com/starrocks/qe/StmtExecutor.java | 6 +- .../java/com/starrocks/sql/DeletePlanner.java | 2 +- .../sql/analyzer/DeleteAnalyzer.java | 115 ++++++++++++-- .../com/starrocks/sql/ast/DeleteStmt.java | 125 +++------------ .../com/starrocks/sql/parser/AstBuilder.java | 3 +- .../com/starrocks/sql/parser/StarRocks.g4 | 2 +- .../starrocks/analysis/DeleteStmtTest.java | 146 +----------------- .../com/starrocks/lake/delete/DeleteTest.java | 16 +- .../com/starrocks/load/DeleteHandlerTest.java | 29 ++-- .../sql/analyzer/AnalyzeDeleteTest.java | 39 ++++- 11 files changed, 191 insertions(+), 294 deletions(-) diff --git a/fe/fe-core/src/main/java/com/starrocks/load/DeleteHandler.java b/fe/fe-core/src/main/java/com/starrocks/load/DeleteHandler.java index 6d22e25d572935..a092d09f7937fd 100644 --- a/fe/fe-core/src/main/java/com/starrocks/load/DeleteHandler.java +++ b/fe/fe-core/src/main/java/com/starrocks/load/DeleteHandler.java @@ -194,7 +194,7 @@ private DeleteJob createJob(DeleteStmt stmt, List conditions, Databas } // get partitions - List partitionNames = stmt.getPartitionNames(); + List partitionNames = stmt.getPartitionNamesList(); Preconditions.checkState(partitionNames != null); boolean noPartitionSpecified = partitionNames.isEmpty(); if (noPartitionSpecified) { diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java b/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java index 5d68dda4bdf36e..afe5cf33693835 100644 --- a/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java +++ b/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java @@ -642,7 +642,7 @@ public void analyze(TQueryOptions tQueryOptions) throws UserException { // Because this is called by other thread public void cancel() { - if (parsedStmt instanceof DeleteStmt && !((DeleteStmt) parsedStmt).supportNewPlanner()) { + if (parsedStmt instanceof DeleteStmt && ((DeleteStmt) parsedStmt).shouldHandledByDeleteHandler()) { DeleteStmt deleteStmt = (DeleteStmt) parsedStmt; long jobId = deleteStmt.getJobId(); if (jobId != -1) { @@ -673,7 +673,7 @@ private void handleKill() throws DdlException { // Only user itself and user with admin priv can kill connection if (!killCtx.getQualifiedUser().equals(ConnectContext.get().getQualifiedUser()) && !GlobalStateMgr.getCurrentState().getAuth().checkGlobalPriv(ConnectContext.get(), - PrivPredicate.ADMIN)) { + PrivPredicate.ADMIN)) { ErrorReport.reportDdlException(ErrorCode.ERR_KILL_DENIED_ERROR, id); } } @@ -1183,7 +1183,7 @@ public void handleDMLStmt(ExecPlan execPlan, DmlStmt stmt) throws Exception { } // special handling for delete of non-primary key table, using old handler - if (stmt instanceof DeleteStmt && !((DeleteStmt) stmt).supportNewPlanner()) { + if (stmt instanceof DeleteStmt && ((DeleteStmt) stmt).shouldHandledByDeleteHandler()) { try { context.getGlobalStateMgr().getDeleteHandler().process((DeleteStmt) stmt); context.getState().setOk(); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/DeletePlanner.java b/fe/fe-core/src/main/java/com/starrocks/sql/DeletePlanner.java index 806f8f58e90c15..732f0c21039043 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/DeletePlanner.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/DeletePlanner.java @@ -31,7 +31,7 @@ public class DeletePlanner { public ExecPlan plan(DeleteStmt deleteStatement, ConnectContext session) { - if (!deleteStatement.supportNewPlanner()) { + if (deleteStatement.shouldHandledByDeleteHandler()) { // executor will use DeleteHandler to handle delete statement // so just return empty plan here return null; diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/DeleteAnalyzer.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/DeleteAnalyzer.java index eee1d56d1180d0..8e91c67052ca52 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/DeleteAnalyzer.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/DeleteAnalyzer.java @@ -1,8 +1,16 @@ // This file is licensed under the Elastic License 2.0. Copyright 2021-present, StarRocks Inc. package com.starrocks.sql.analyzer; -import com.starrocks.analysis.Analyzer; +import com.google.common.base.Strings; +import com.google.common.collect.Lists; +import com.starrocks.analysis.BinaryPredicate; +import com.starrocks.analysis.CompoundPredicate; +import com.starrocks.analysis.Expr; +import com.starrocks.analysis.InPredicate; import com.starrocks.analysis.IntLiteral; +import com.starrocks.analysis.IsNullPredicate; +import com.starrocks.analysis.LiteralExpr; +import com.starrocks.analysis.Predicate; import com.starrocks.analysis.SlotRef; import com.starrocks.analysis.TableName; import com.starrocks.catalog.Column; @@ -11,10 +19,14 @@ import com.starrocks.catalog.OlapTable; import com.starrocks.catalog.Table; import com.starrocks.catalog.Type; +import com.starrocks.common.Config; import com.starrocks.load.Load; import com.starrocks.qe.ConnectContext; import com.starrocks.sql.ast.DeleteStmt; +import com.starrocks.sql.ast.JoinRelation; +import com.starrocks.sql.ast.PartitionNames; import com.starrocks.sql.ast.QueryStatement; +import com.starrocks.sql.ast.Relation; import com.starrocks.sql.ast.SelectList; import com.starrocks.sql.ast.SelectListItem; import com.starrocks.sql.ast.SelectRelation; @@ -23,9 +35,92 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.List; + public class DeleteAnalyzer { private static final Logger LOG = LogManager.getLogger(DeleteAnalyzer.class); + private static void analyzePredicate(Expr predicate, List deleteConditions) { + if (predicate instanceof BinaryPredicate) { + BinaryPredicate binaryPredicate = (BinaryPredicate) predicate; + Expr leftExpr = binaryPredicate.getChild(0); + if (!(leftExpr instanceof SlotRef)) { + throw new SemanticException("Left expr of binary predicate should be column name"); + } + Expr rightExpr = binaryPredicate.getChild(1); + if (!(rightExpr instanceof LiteralExpr)) { + throw new SemanticException("Right expr of binary predicate should be value"); + } + deleteConditions.add(binaryPredicate); + } else if (predicate instanceof CompoundPredicate) { + CompoundPredicate compoundPredicate = (CompoundPredicate) predicate; + if (compoundPredicate.getOp() != CompoundPredicate.Operator.AND) { + throw new SemanticException("Compound predicate's op should be AND"); + } + + analyzePredicate(compoundPredicate.getChild(0), deleteConditions); + analyzePredicate(compoundPredicate.getChild(1), deleteConditions); + } else if (predicate instanceof IsNullPredicate) { + IsNullPredicate isNullPredicate = (IsNullPredicate) predicate; + Expr leftExpr = isNullPredicate.getChild(0); + if (!(leftExpr instanceof SlotRef)) { + throw new SemanticException("Left expr of is_null predicate should be column name"); + } + deleteConditions.add(isNullPredicate); + } else if (predicate instanceof InPredicate) { + InPredicate inPredicate = (InPredicate) predicate; + Expr leftExpr = inPredicate.getChild(0); + if (!(leftExpr instanceof SlotRef)) { + throw new SemanticException("Left expr of binary predicate should be column name"); + } + int inElementNum = inPredicate.getInElementNum(); + int maxAllowedInElementNumOfDelete = Config.max_allowed_in_element_num_of_delete; + if (inElementNum > maxAllowedInElementNumOfDelete) { + throw new SemanticException("Element num of predicate should not be more than " + + maxAllowedInElementNumOfDelete); + } + for (int i = 1; i <= inElementNum; i++) { + Expr expr = inPredicate.getChild(i); + if (!(expr instanceof LiteralExpr)) { + throw new SemanticException("Child of in predicate should be value"); + } + } + deleteConditions.add(inPredicate); + } else { + throw new SemanticException("Where clause only supports compound predicate, binary predicate, " + + "is_null predicate and in predicate"); + } + } + + private static void analyzeNonPrimaryKey(DeleteStmt deleteStatement) { + PartitionNames partitionNames = deleteStatement.getPartitionNames(); + if (partitionNames != null) { + if (partitionNames.isTemp()) { + throw new SemanticException("Do not support deleting temp partitions"); + } + List names = partitionNames.getPartitionNames(); + if (names.isEmpty()) { + throw new SemanticException("No partition specifed in partition lists"); + } + // check if partition name is not empty string + if (names.stream().anyMatch(entity -> Strings.isNullOrEmpty(entity))) { + throw new SemanticException("there are empty partition name"); + } + } + + if (deleteStatement.getUsingRelations() != null) { + throw new SemanticException("Do not support `using` clause in non-primary table"); + } + + if (deleteStatement.getWherePredicate() == null) { + throw new SemanticException("Where clause is not set"); + } + + List deleteConditions = Lists.newLinkedList(); + analyzePredicate(deleteStatement.getWherePredicate(), deleteConditions); + deleteStatement.setDeleteConditions(deleteConditions); + } + public static void analyze(DeleteStmt deleteStatement, ConnectContext session) { TableName tableName = deleteStatement.getTableName(); MetaUtils.normalizationTableName(session, tableName); @@ -40,12 +135,7 @@ public static void analyze(DeleteStmt deleteStatement, ConnectContext session) { } if (!(table instanceof OlapTable && ((OlapTable) table).getKeysType() == KeysType.PRIMARY_KEYS)) { - try { - deleteStatement.analyze(new Analyzer(session.getGlobalStateMgr(), session)); - } catch (Exception e) { - LOG.warn("Analyze DeleteStmt using old analyzer failed", e); - throw new SemanticException("Analyze DeleteStmt using old analyzer failed: " + e.getMessage()); - } + analyzeNonPrimaryKey(deleteStatement); return; } @@ -53,7 +143,7 @@ public static void analyze(DeleteStmt deleteStatement, ConnectContext session) { if (deleteStatement.getWherePredicate() == null) { throw new SemanticException("Delete must specify where clause to prevent full table delete"); } - if (deleteStatement.getPartitionNames() != null && deleteStatement.getPartitionNames().size() > 0) { + if (deleteStatement.getPartitionNamesList() != null && deleteStatement.getPartitionNamesList().size() > 0) { throw new SemanticException("Delete for primary key table do not support specifying partitions"); } @@ -73,9 +163,14 @@ public static void analyze(DeleteStmt deleteStatement, ConnectContext session) { throw new SemanticException("analyze delete failed", e); } - TableRelation tableRelation = new TableRelation(tableName); + Relation relation = new TableRelation(tableName); + if (deleteStatement.getUsingRelations() != null) { + for (Relation r : deleteStatement.getUsingRelations()) { + relation = new JoinRelation(null, relation, r, null, false); + } + } SelectRelation selectRelation = - new SelectRelation(selectList, tableRelation, deleteStatement.getWherePredicate(), null, null); + new SelectRelation(selectList, relation, deleteStatement.getWherePredicate(), null, null); QueryStatement queryStatement = new QueryStatement(selectRelation); queryStatement.setIsExplain(deleteStatement.isExplain(), deleteStatement.getExplainLevel()); new QueryAnalyzer(session).analyze(queryStatement); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/ast/DeleteStmt.java b/fe/fe-core/src/main/java/com/starrocks/sql/ast/DeleteStmt.java index 5e20c058c26c4d..d06185815f56e9 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/ast/DeleteStmt.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/ast/DeleteStmt.java @@ -21,29 +21,18 @@ package com.starrocks.sql.ast; -import com.google.common.base.Joiner; import com.google.common.collect.Lists; -import com.starrocks.analysis.Analyzer; -import com.starrocks.analysis.BinaryPredicate; -import com.starrocks.analysis.CompoundPredicate; -import com.starrocks.analysis.CompoundPredicate.Operator; import com.starrocks.analysis.Expr; -import com.starrocks.analysis.InPredicate; -import com.starrocks.analysis.IsNullPredicate; -import com.starrocks.analysis.LiteralExpr; import com.starrocks.analysis.Predicate; -import com.starrocks.analysis.SlotRef; import com.starrocks.analysis.TableName; import com.starrocks.catalog.Table; -import com.starrocks.common.AnalysisException; -import com.starrocks.common.Config; -import com.starrocks.common.UserException; import java.util.List; public class DeleteStmt extends DmlStmt { private final TableName tblName; private final PartitionNames partitionNames; + private final List usingRelations; private final Expr wherePredicate; // fields for new planer, primary key table @@ -51,14 +40,19 @@ public class DeleteStmt extends DmlStmt { private QueryStatement queryStatement; // fields for old planer, non-primary key table - private final List deleteConditions; + private List deleteConditions; // Each deleteStmt corresponds to a DeleteJob. // The JobID is generated here for easy correlation when cancel Delete private long jobId = -1; public DeleteStmt(TableName tableName, PartitionNames partitionNames, Expr wherePredicate) { + this(tableName, partitionNames, null, wherePredicate); + } + + public DeleteStmt(TableName tableName, PartitionNames partitionNames, List usingRelations, Expr wherePredicate) { this.tblName = tableName; this.partitionNames = partitionNames; + this.usingRelations = usingRelations; this.wherePredicate = wherePredicate; this.deleteConditions = Lists.newLinkedList(); } @@ -80,105 +74,30 @@ public Expr getWherePredicate() { return wherePredicate; } - public List getPartitionNames() { + public List getPartitionNamesList() { return partitionNames == null ? Lists.newArrayList() : partitionNames.getPartitionNames(); } + public PartitionNames getPartitionNames() { + return partitionNames; + } + + public List getUsingRelations() { + return usingRelations; + } + public List getDeleteConditions() { return deleteConditions; } - @Override - public void analyze(Analyzer analyzer) throws UserException { - if (tblName == null) { - throw new AnalysisException("Table is not set"); - } - - tblName.analyze(analyzer); - - if (partitionNames != null) { - partitionNames.analyze(analyzer); - if (partitionNames.isTemp()) { - throw new AnalysisException("Do not support deleting temp partitions"); - } - } - - if (wherePredicate == null) { - throw new AnalysisException("Where clause is not set"); - } - - // analyze predicate - analyzePredicate(wherePredicate); - } - - private void analyzePredicate(Expr predicate) throws AnalysisException { - if (predicate instanceof BinaryPredicate) { - BinaryPredicate binaryPredicate = (BinaryPredicate) predicate; - Expr leftExpr = binaryPredicate.getChild(0); - if (!(leftExpr instanceof SlotRef)) { - throw new AnalysisException("Left expr of binary predicate should be column name"); - } - Expr rightExpr = binaryPredicate.getChild(1); - if (!(rightExpr instanceof LiteralExpr)) { - throw new AnalysisException("Right expr of binary predicate should be value"); - } - deleteConditions.add(binaryPredicate); - } else if (predicate instanceof CompoundPredicate) { - CompoundPredicate compoundPredicate = (CompoundPredicate) predicate; - if (compoundPredicate.getOp() != Operator.AND) { - throw new AnalysisException("Compound predicate's op should be AND"); - } - - analyzePredicate(compoundPredicate.getChild(0)); - analyzePredicate(compoundPredicate.getChild(1)); - } else if (predicate instanceof IsNullPredicate) { - IsNullPredicate isNullPredicate = (IsNullPredicate) predicate; - Expr leftExpr = isNullPredicate.getChild(0); - if (!(leftExpr instanceof SlotRef)) { - throw new AnalysisException("Left expr of is_null predicate should be column name"); - } - deleteConditions.add(isNullPredicate); - } else if (predicate instanceof InPredicate) { - InPredicate inPredicate = (InPredicate) predicate; - Expr leftExpr = inPredicate.getChild(0); - if (!(leftExpr instanceof SlotRef)) { - throw new AnalysisException("Left expr of binary predicate should be column name"); - } - int inElementNum = inPredicate.getInElementNum(); - int maxAllowedInElementNumOfDelete = Config.max_allowed_in_element_num_of_delete; - if (inElementNum > maxAllowedInElementNumOfDelete) { - throw new AnalysisException("Element num of predicate should not be more than " + - maxAllowedInElementNumOfDelete); - } - for (int i = 1; i <= inElementNum; i++) { - Expr expr = inPredicate.getChild(i); - if (!(expr instanceof LiteralExpr)) { - throw new AnalysisException("Child of in predicate should be value"); - } - } - deleteConditions.add(inPredicate); - } else { - throw new AnalysisException("Where clause only supports compound predicate, binary predicate, " + - "is_null predicate and in predicate"); - } + public void setDeleteConditions(List deleteConditions) { + this.deleteConditions = deleteConditions; } - @Override - public String toSql() { - StringBuilder sb = new StringBuilder(); - sb.append("DELETE FROM ").append(tblName.toSql()); - if (partitionNames != null) { - sb.append(" PARTITION ("); - sb.append(Joiner.on(", ").join(partitionNames.getPartitionNames())); - sb.append(")"); - } - sb.append(" WHERE ").append(wherePredicate.toSql()); - return sb.toString(); - } - - public boolean supportNewPlanner() { - // table must present if analyzed by new analyzer - return table != null; + public boolean shouldHandledByDeleteHandler() { + // table must present if analyzed and is a delele for primary key table, so it is executed by new + // planner&execution engine, otherwise(non-pk table) it is handled by DeleteHandler + return table == null; } public void setTable(Table table) { 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 95bd3e8df023d6..bb0716f6f74e28 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 @@ -1341,8 +1341,9 @@ public ParseNode visitDeleteStatement(StarRocksParser.DeleteStatementContext con if (context.partitionNames() != null) { partitionNames = (PartitionNames) visit(context.partitionNames()); } + List usingRelations = context.using != null ? visit(context.using.relation(), Relation.class) : null; Expr where = context.where != null ? (Expr) visit(context.where) : null; - DeleteStmt ret = new DeleteStmt(targetTableName, partitionNames, where); + DeleteStmt ret = new DeleteStmt(targetTableName, partitionNames, usingRelations, where); if (context.explainDesc() != null) { ret.setIsExplain(true, getExplainType(context.explainDesc())); } diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocks.g4 b/fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocks.g4 index 4ec454f3f9938e..63de30231881c6 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocks.g4 +++ b/fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocks.g4 @@ -755,7 +755,7 @@ updateStatement ; deleteStatement - : explainDesc? DELETE FROM qualifiedName partitionNames? (WHERE where=expression)? + : explainDesc? DELETE FROM qualifiedName partitionNames? (USING using=relations)? (WHERE where=expression)? ; // ------------------------------------------- Routine Statement ----------------------------------------------------------- diff --git a/fe/fe-core/src/test/java/com/starrocks/analysis/DeleteStmtTest.java b/fe/fe-core/src/test/java/com/starrocks/analysis/DeleteStmtTest.java index 21997f6ad19e99..731737964bd3b9 100644 --- a/fe/fe-core/src/test/java/com/starrocks/analysis/DeleteStmtTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/analysis/DeleteStmtTest.java @@ -23,7 +23,6 @@ import com.google.common.collect.Lists; import com.starrocks.analysis.BinaryPredicate.Operator; -import com.starrocks.common.UserException; import com.starrocks.mysql.privilege.Auth; import com.starrocks.mysql.privilege.MockedAuth; import com.starrocks.qe.ConnectContext; @@ -34,12 +33,7 @@ import org.junit.Before; import org.junit.Test; -import java.util.List; - public class DeleteStmtTest { - - Analyzer analyzer; - @Mocked private Auth auth; @Mocked @@ -47,7 +41,6 @@ public class DeleteStmtTest { @Before public void setUp() { - analyzer = AccessTestUtil.fetchAdminAnalyzer(); MockedAuth.mockedAuth(auth); MockedAuth.mockedConnectContext(ctx, "root", "192.168.1.1"); } @@ -61,143 +54,6 @@ public void getMethodTest() { Assert.assertEquals("testDb", deleteStmt.getTableName().getDb()); Assert.assertEquals("testTbl", deleteStmt.getTableName().getTbl()); - Assert.assertEquals(Lists.newArrayList("partition"), deleteStmt.getPartitionNames()); - Assert.assertEquals("DELETE FROM `testDb`.`testTbl` PARTITION (partition) WHERE `k1` = 'abc'", - deleteStmt.toSql()); - } - - @Test - public void testAnalyze() { - // case 1 - LikePredicate likePredicate = new LikePredicate(com.starrocks.analysis.LikePredicate.Operator.LIKE, - new SlotRef(null, "k1"), - new StringLiteral("abc")); - DeleteStmt deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition")), likePredicate); - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - Assert.assertTrue(e.getMessage().contains("Where clause only supports compound predicate, " + - "binary predicate, is_null predicate and in predicate")); - } - - // case 2 - BinaryPredicate binaryPredicate = new BinaryPredicate(Operator.EQ, new SlotRef(null, "k1"), - new StringLiteral("abc")); - CompoundPredicate compoundPredicate = - new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.OR, binaryPredicate, - binaryPredicate); - - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition")), compoundPredicate); - - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - Assert.assertTrue(e.getMessage().contains("should be AND")); - } - - // case 3 - compoundPredicate = new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.AND, - binaryPredicate, - likePredicate); - - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition")), compoundPredicate); - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - Assert.assertTrue(e.getMessage().contains("Where clause only supports compound predicate, " + - "binary predicate, is_null predicate and in predicate")); - } - - // case 4 - binaryPredicate = new BinaryPredicate(Operator.EQ, new SlotRef(null, "k1"), - new SlotRef(null, "k1")); - compoundPredicate = new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.AND, - binaryPredicate, - binaryPredicate); - - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition")), compoundPredicate); - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - Assert.assertTrue(e.getMessage().contains("Right expr of binary predicate should be value")); - } - - // case 5 - binaryPredicate = new BinaryPredicate(Operator.EQ, new StringLiteral("abc"), - new SlotRef(null, "k1")); - compoundPredicate = new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.AND, - binaryPredicate, - binaryPredicate); - - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition")), compoundPredicate); - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - Assert.assertTrue(e.getMessage().contains("Left expr of binary predicate should be column name")); - } - - // case 6 partition is null - binaryPredicate = new BinaryPredicate(Operator.EQ, new SlotRef(null, "k1"), new StringLiteral("abc")); - compoundPredicate = new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.AND, - binaryPredicate, - binaryPredicate); - - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), null, compoundPredicate); - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - e.printStackTrace(); - Assert.assertTrue(e.getMessage().contains("Partition is not set")); - } - - // normal - binaryPredicate = new BinaryPredicate(Operator.EQ, new SlotRef(null, "k1"), - new StringLiteral("abc")); - List inList = Lists.newArrayList(); - inList.add(new StringLiteral("1234")); - inList.add(new StringLiteral("5678")); - inList.add(new StringLiteral("1314")); - inList.add(new StringLiteral("8972")); - InPredicate inPredicate = new InPredicate(new SlotRef(null, "k2"), inList, true); - CompoundPredicate compoundPredicate2 = - new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.AND, - binaryPredicate, - inPredicate); - compoundPredicate = new CompoundPredicate(com.starrocks.analysis.CompoundPredicate.Operator.AND, - binaryPredicate, - compoundPredicate2); - - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition")), compoundPredicate); - try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { - Assert.fail(); - } - - // multi partition - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), - new PartitionNames(false, Lists.newArrayList("partition1", "partition2")), compoundPredicate); - try { - deleteStmt.analyze(analyzer); - Assert.assertEquals(Lists.newArrayList("partition1", "partition2"), deleteStmt.getPartitionNames()); - } catch (UserException e) { - Assert.fail(); - } - - // no partition - deleteStmt = new DeleteStmt(new TableName("testDb", "testTbl"), null, compoundPredicate); - try { - deleteStmt.analyze(analyzer); - Assert.assertEquals(Lists.newArrayList(), deleteStmt.getPartitionNames()); - } catch (UserException e) { - Assert.fail(); - } + Assert.assertEquals(Lists.newArrayList("partition"), deleteStmt.getPartitionNamesList()); } - } diff --git a/fe/fe-core/src/test/java/com/starrocks/lake/delete/DeleteTest.java b/fe/fe-core/src/test/java/com/starrocks/lake/delete/DeleteTest.java index 8b5965d3819fa4..0bbc10caba9e80 100644 --- a/fe/fe-core/src/test/java/com/starrocks/lake/delete/DeleteTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/lake/delete/DeleteTest.java @@ -4,7 +4,6 @@ import com.google.common.collect.Lists; import com.starrocks.analysis.AccessTestUtil; -import com.starrocks.analysis.Analyzer; import com.starrocks.analysis.BinaryPredicate; import com.starrocks.analysis.IntLiteral; import com.starrocks.analysis.SlotRef; @@ -33,6 +32,7 @@ import com.starrocks.persist.EditLog; import com.starrocks.proto.DeleteDataRequest; import com.starrocks.proto.DeleteDataResponse; +import com.starrocks.qe.ConnectContext; import com.starrocks.qe.QueryStateException; import com.starrocks.rpc.BrpcProxy; import com.starrocks.rpc.LakeService; @@ -89,7 +89,7 @@ public class DeleteTest { private Database db; private Auth auth; - private Analyzer analyzer; + private ConnectContext connectContext = new ConnectContext(); private DeleteHandler deleteHandler; private Database createDb() { @@ -153,9 +153,9 @@ public void setUpExpectation() { @Before public void setUp() { + connectContext.setGlobalStateMgr(globalStateMgr); deleteHandler = new DeleteHandler(); auth = AccessTestUtil.fetchAdminAccess(); - analyzer = AccessTestUtil.fetchAdminAnalyzer(); db = createDb(); } @@ -217,8 +217,8 @@ public DeleteDataResponse get(long timeout, @NotNull TimeUnit unit) new PartitionNames(false, Lists.newArrayList(partitionName)), binaryPredicate); try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } @@ -283,8 +283,8 @@ public DeleteDataResponse get(long timeout, @NotNull TimeUnit unit) new PartitionNames(false, Lists.newArrayList(partitionName)), binaryPredicate); try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } @@ -324,7 +324,7 @@ public LakeService getLakeService(String host, int port) { DeleteStmt deleteStmt = new DeleteStmt(new TableName(dbName, tableName), new PartitionNames(false, Lists.newArrayList(partitionName)), binaryPredicate); - deleteStmt.analyze(analyzer); + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); try { deleteHandler.process(deleteStmt); } catch (DdlException e) { diff --git a/fe/fe-core/src/test/java/com/starrocks/load/DeleteHandlerTest.java b/fe/fe-core/src/test/java/com/starrocks/load/DeleteHandlerTest.java index 8f8baf28106c65..2f9c20c497aa88 100644 --- a/fe/fe-core/src/test/java/com/starrocks/load/DeleteHandlerTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/load/DeleteHandlerTest.java @@ -5,7 +5,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.starrocks.analysis.AccessTestUtil; -import com.starrocks.analysis.Analyzer; import com.starrocks.analysis.BinaryPredicate; import com.starrocks.analysis.IntLiteral; import com.starrocks.analysis.SlotRef; @@ -83,8 +82,6 @@ public class DeleteHandlerTest { private Database db; private Auth auth; - Analyzer analyzer; - private GlobalTransactionMgr globalTransactionMgr; private TabletInvertedIndex invertedIndex = new TabletInvertedIndex(); private ConnectContext connectContext = new ConnectContext(); @@ -95,9 +92,9 @@ public void setUp() { globalTransactionMgr = new GlobalTransactionMgr(globalStateMgr); globalTransactionMgr.setEditLog(editLog); + connectContext.setGlobalStateMgr(globalStateMgr); deleteHandler = new DeleteHandler(); auth = AccessTestUtil.fetchAdminAccess(); - analyzer = AccessTestUtil.fetchAdminAnalyzer(); try { db = CatalogMocker.mockDb(); } catch (AnalysisException e) { @@ -204,8 +201,8 @@ public void testUnQuorumTimeout() throws DdlException, QueryStateException { } }; try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } deleteHandler.process(deleteStmt); @@ -250,8 +247,8 @@ public TransactionState getTransactionState(long transactionId) { }; try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } try { @@ -306,8 +303,8 @@ public TransactionState getTransactionState(long transactionId) { }; try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } @@ -369,8 +366,8 @@ public Collection getTabletDeleteInfo() { }; try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } try { @@ -436,8 +433,8 @@ public Collection getTabletDeleteInfo() { }; try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } try { @@ -490,8 +487,8 @@ public Collection getTabletDeleteInfo() { }; try { - deleteStmt.analyze(analyzer); - } catch (UserException e) { + com.starrocks.sql.analyzer.Analyzer.analyze(deleteStmt, connectContext); + } catch (Exception e) { Assert.fail(); } try { diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeDeleteTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeDeleteTest.java index 9a6e6d8b3018f6..ff180b8c57e168 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeDeleteTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeDeleteTest.java @@ -35,23 +35,43 @@ public static void tearDown() { public void testPartitions() { DeleteStmt st; st = (DeleteStmt) SqlParser.parse("delete from tjson partition (p0)", 0).get(0); - Assert.assertEquals(1, st.getPartitionNames().size()); + Assert.assertEquals(1, st.getPartitionNamesList().size()); st = (DeleteStmt) SqlParser.parse("delete from tjson partition p0", 0).get(0); - Assert.assertEquals(1, st.getPartitionNames().size()); + Assert.assertEquals(1, st.getPartitionNamesList().size()); st = (DeleteStmt) SqlParser.parse("delete from tjson partition (p0, p1)", 0).get(0); - Assert.assertEquals(2, st.getPartitionNames().size()); + Assert.assertEquals(2, st.getPartitionNamesList().size()); + } + + @Test + public void testAnalyzeNonPrimaryKey() { + analyzeFail("delete from tjson where v_json like 'abc'", + "Where clause only supports"); + + analyzeFail("delete from tjson where v_int > 20 and v_json like 'abc'", + "Where clause only supports"); + + analyzeFail("delete from tjson where v_int < 10 or v_int > 20", + "should be AND"); + + analyzeFail("delete from tjson where v_int = v_int and v_int = v_int", + "Right expr of binary predicate should be value"); + + analyzeFail("delete from tjson where 10 = v_int and 10 = v_int", + "Left expr of binary predicate should be column name"); + + analyzeSuccess("delete from tjson where v_json in ('1','2','3') and v_int > 10 and v_int < 40"); } @Test public void testSingle() { StatementBase stmt = analyzeSuccess("delete from tjson where v_int = 1"); - Assert.assertEquals(false, ((DeleteStmt) stmt).supportNewPlanner()); + Assert.assertEquals(true, ((DeleteStmt) stmt).shouldHandledByDeleteHandler()); analyzeFail("delete from tjson", "Where clause is not set"); stmt = analyzeSuccess("delete from tprimary where pk = 1"); - Assert.assertEquals(true, ((DeleteStmt) stmt).supportNewPlanner()); + Assert.assertEquals(false, ((DeleteStmt) stmt).shouldHandledByDeleteHandler()); analyzeFail("delete from tprimary partitions (p1, p2) where pk = 1", "Delete for primary key table do not support specifying partitions"); @@ -59,4 +79,13 @@ public void testSingle() { analyzeFail("delete from tprimary", "Delete must specify where clause to prevent full table delete"); } + + @Test + public void testUsing() { + analyzeSuccess("delete from tprimary using tprimary2 tp2 where tprimary.pk = tp2.pk"); + + analyzeSuccess( + "delete from tprimary using tprimary2 tp2 join t0 where tprimary.pk = tp2.pk " + + "and tp2.pk = t0.v1 and t0.v2 > 0"); + } }