From a9596d684bd05dc0f7d19846d42136357010e64a Mon Sep 17 00:00:00 2001 From: yangzhg Date: Tue, 1 Sep 2020 16:43:18 +0800 Subject: [PATCH 1/6] fix semi/anti join error when table has delete sign column --- .../apache/doris/analysis/BaseTableRef.java | 5 ++ .../org/apache/doris/analysis/SelectStmt.java | 45 ++++++++++++------ .../apache/doris/analysis/SelectStmtTest.java | 46 ++++++++++++++++++- 3 files changed, 80 insertions(+), 16 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java index 3eceea0fbb3949..7dbce382171b53 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java @@ -52,6 +52,11 @@ public TableRef clone() { return new BaseTableRef(this); } + @Override + public Table getTable() { + return table; + } + @Override public TupleDescriptor createTupleDescriptor(Analyzer analyzer) { TupleDescriptor result = analyzer.getDescTbl().createTupleDescriptor(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java index f640c8d456f89d..e2b5778a7f9b06 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java @@ -340,7 +340,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { return; } super.analyze(analyzer); - + whereClauseRewrite(); fromClause_.setNeedToSql(needToSql); fromClause_.analyze(analyzer); @@ -439,7 +439,6 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { } // do this before whereClause.analyze , some expr is not analyzed, this may cause some // function not work as expected such as equals; - whereClauseRewrite(); if (whereClause != null) { if (checkGroupingFn(whereClause)) { throw new AnalysisException("grouping operations are not allowed in WHERE."); @@ -525,29 +524,45 @@ public List collectTupleIds() { return result; } - private void whereClauseRewrite() { - if (whereClause instanceof IntLiteral) { - if (((IntLiteral) whereClause).getLongValue() == 0) { - whereClause = new BoolLiteral(false); - } else { - whereClause = new BoolLiteral(true); - } - } - // filter deleted data by and DELETE_SIGN column = 0, DELETE_SIGN is a hidden column - // that indicates whether the row is delete + private Expr filterDeleteSignCol(Expr origExpr) throws AnalysisException { + Expr expr = origExpr; for (TableRef tableRef : fromClause_.getTableRefs()) { + tableRef = analyzer.resolveTableRef(tableRef); if (!isForbiddenMVRewrite() && tableRef instanceof BaseTableRef && tableRef.getTable() instanceof OlapTable && ((OlapTable) tableRef.getTable()).getKeysType() == KeysType.UNIQUE_KEYS && ((OlapTable) tableRef.getTable()).hasDeleteSign()) { BinaryPredicate filterDeleteExpr = new BinaryPredicate(BinaryPredicate.Operator.EQ, new SlotRef(tableRef.getName(), Column.DELETE_SIGN), new IntLiteral(0)); - if (whereClause == null) { - whereClause = filterDeleteExpr; + if (expr == null) { + expr = filterDeleteExpr; } else { - whereClause = new CompoundPredicate(CompoundPredicate.Operator.AND, filterDeleteExpr, whereClause); + expr = new CompoundPredicate(CompoundPredicate.Operator.AND, filterDeleteExpr, expr); } } } + return expr; + } + + private void whereClauseRewrite() throws AnalysisException { + if (whereClause instanceof IntLiteral) { + if (((IntLiteral) whereClause).getLongValue() == 0) { + whereClause = new BoolLiteral(false); + } else { + whereClause = new BoolLiteral(true); + } + } + // filter deleted data by and DELETE_SIGN column = 0, DELETE_SIGN is a hidden column + // that indicates whether the row is delete + if (fromClause_.getTableRefs().size() == 2 + && (fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.LEFT_SEMI_JOIN + || fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.RIGHT_SEMI_JOIN + || fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.LEFT_ANTI_JOIN + || fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.RIGHT_ANTI_JOIN)) { + fromClause_.getTableRefs().get(1).setOnClause( + filterDeleteSignCol(fromClause_.getTableRefs().get(1).getOnClause())); + } else { + whereClause = filterDeleteSignCol(whereClause); + } Expr deDuplicatedWhere = deduplicateOrs(whereClause); if (deDuplicatedWhere != null) { whereClause = deDuplicatedWhere; diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java index 278e69d64eceeb..ab7c96ebf5760b 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java @@ -64,11 +64,42 @@ public static void setUp() throws Exception { "\"storage_type\" = \"COLUMN\",\n" + "\"replication_num\" = \"1\"\n" + ");"; + + String tbl1 = "CREATE TABLE db1.table1 (\n" + + " `siteid` int(11) NULL DEFAULT \"10\" COMMENT \"\",\n" + + " `citycode` smallint(6) NULL COMMENT \"\",\n" + + " `username` varchar(32) NULL DEFAULT \"\" COMMENT \"\",\n" + + " `pv` bigint(20) NULL DEFAULT \"0\" COMMENT \"\"\n" + + ") ENGINE=OLAP\n" + + "UNIQUE KEY(`siteid`, `citycode`, `username`)\n" + + "COMMENT \"OLAP\"\n" + + "DISTRIBUTED BY HASH(`siteid`) BUCKETS 10\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"in_memory\" = \"false\",\n" + + "\"storage_format\" = \"V2\"\n" + + ")"; + String tbl2 = "CREATE TABLE db1.table2 (\n" + + " `siteid` int(11) NULL DEFAULT \"10\" COMMENT \"\",\n" + + " `citycode` smallint(6) NULL COMMENT \"\",\n" + + " `username` varchar(32) NULL DEFAULT \"\" COMMENT \"\",\n" + + " `pv` bigint(20) NULL DEFAULT \"0\" COMMENT \"\"\n" + + ") ENGINE=OLAP\n" + + "UNIQUE KEY(`siteid`, `citycode`, `username`)\n" + + "COMMENT \"OLAP\"\n" + + "DISTRIBUTED BY HASH(`siteid`) BUCKETS 10\n" + + "PROPERTIES (\n" + + "\"replication_num\" = \"1\",\n" + + "\"in_memory\" = \"false\",\n" + + "\"storage_format\" = \"V2\"\n" + + ")"; dorisAssert = new DorisAssert(); dorisAssert.withDatabase("db1").useDatabase("db1"); dorisAssert.withTable(createTblStmtStr) .withTable(createBaseAllStmtStr) - .withTable(createPratitionTableStr); + .withTable(createPratitionTableStr) + .withTable(tbl1) + .withTable(tbl2); } @Test @@ -396,4 +427,17 @@ public void testVarcharToLongSupport() throws Exception { .explainQuery() .contains("`datekey` = 20200730")); } + @Test + public void testDeleteSign() throws Exception { + ConnectContext ctx = UtFrameUtils.createDefaultCtx(); + String sql1 = "SELECT * FROM db1.table1 LEFT ANTI JOIN db1.table2 ON db1.table1.siteid = db1.table2.siteid;"; + SelectStmt stmt1 = (SelectStmt) UtFrameUtils.parseAndAnalyzeStmt(sql1, ctx); + Assert.assertTrue(stmt1.toSql().contains("ON (`default_cluster:db1`.`table2`.`__DORIS_DELETE_SIGN__` = 0) " + + "AND ((`default_cluster:db1`.`table1`.`__DORIS_DELETE_SIGN__` = 0) AND " + + "(`db1`.`table1`.`siteid` = `db1`.`table2`.`siteid`))")); + String sql2 = "SELECT * FROM db1.table1 JOIN db1.table2 ON db1.table1.siteid = db1.table2.siteid;"; + SelectStmt stmt2 = (SelectStmt) UtFrameUtils.parseAndAnalyzeStmt(sql2, ctx); + Assert.assertTrue(stmt2.toSql().contains(" WHERE (`default_cluster:db1`.`table2`.`__DORIS_DELETE_SIGN__` = 0)" + + " AND (`default_cluster:db1`.`table1`.`__DORIS_DELETE_SIGN__` = 0)")); + } } From 0f0cc63ea855b61279ce84a10eb193b6bc7a592f Mon Sep 17 00:00:00 2001 From: yangzhg Date: Tue, 1 Sep 2020 19:54:27 +0800 Subject: [PATCH 2/6] move filter to scanNode --- .../org/apache/doris/analysis/Analyzer.java | 2 +- .../org/apache/doris/analysis/SelectStmt.java | 38 +------------------ .../doris/planner/SingleNodePlanner.java | 12 +++++- .../apache/doris/analysis/SelectStmtTest.java | 14 +++---- 4 files changed, 19 insertions(+), 47 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java index 10c82a906c4930..7ded9ba56c1a7f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java @@ -655,7 +655,7 @@ private TupleDescriptor resolveColumnRef(TableName tblName, String colName) thro // find table all name for (TupleDescriptor desc : tupleByAlias.get(tblName.toString())) { //result = desc; - if (!isVisible(desc.getId())) { + if (!colName.equalsIgnoreCase(Column.DELETE_SIGN) && !isVisible(desc.getId())) { ErrorReport.reportAnalysisException(ErrorCode.ERR_ILLEGAL_COLUMN_REFERENCE_ERROR, Joiner.on(".").join(tblName.getTbl(),colName)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java index e2b5778a7f9b06..e69c0bac6bf32a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java @@ -22,7 +22,6 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Database; import org.apache.doris.catalog.FunctionSet; -import org.apache.doris.catalog.KeysType; import org.apache.doris.catalog.OlapTable; import org.apache.doris.catalog.Table.TableType; import org.apache.doris.catalog.Type; @@ -340,7 +339,6 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { return; } super.analyze(analyzer); - whereClauseRewrite(); fromClause_.setNeedToSql(needToSql); fromClause_.analyze(analyzer); @@ -439,6 +437,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException { } // do this before whereClause.analyze , some expr is not analyzed, this may cause some // function not work as expected such as equals; + whereClauseRewrite(); if (whereClause != null) { if (checkGroupingFn(whereClause)) { throw new AnalysisException("grouping operations are not allowed in WHERE."); @@ -524,25 +523,6 @@ public List collectTupleIds() { return result; } - private Expr filterDeleteSignCol(Expr origExpr) throws AnalysisException { - Expr expr = origExpr; - for (TableRef tableRef : fromClause_.getTableRefs()) { - tableRef = analyzer.resolveTableRef(tableRef); - if (!isForbiddenMVRewrite() && tableRef instanceof BaseTableRef && tableRef.getTable() instanceof OlapTable - && ((OlapTable) tableRef.getTable()).getKeysType() == KeysType.UNIQUE_KEYS - && ((OlapTable) tableRef.getTable()).hasDeleteSign()) { - BinaryPredicate filterDeleteExpr = new BinaryPredicate(BinaryPredicate.Operator.EQ, - new SlotRef(tableRef.getName(), Column.DELETE_SIGN), new IntLiteral(0)); - if (expr == null) { - expr = filterDeleteExpr; - } else { - expr = new CompoundPredicate(CompoundPredicate.Operator.AND, filterDeleteExpr, expr); - } - } - } - return expr; - } - private void whereClauseRewrite() throws AnalysisException { if (whereClause instanceof IntLiteral) { if (((IntLiteral) whereClause).getLongValue() == 0) { @@ -551,22 +531,6 @@ private void whereClauseRewrite() throws AnalysisException { whereClause = new BoolLiteral(true); } } - // filter deleted data by and DELETE_SIGN column = 0, DELETE_SIGN is a hidden column - // that indicates whether the row is delete - if (fromClause_.getTableRefs().size() == 2 - && (fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.LEFT_SEMI_JOIN - || fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.RIGHT_SEMI_JOIN - || fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.LEFT_ANTI_JOIN - || fromClause_.getTableRefs().get(1).getJoinOp() == JoinOperator.RIGHT_ANTI_JOIN)) { - fromClause_.getTableRefs().get(1).setOnClause( - filterDeleteSignCol(fromClause_.getTableRefs().get(1).getOnClause())); - } else { - whereClause = filterDeleteSignCol(whereClause); - } - Expr deDuplicatedWhere = deduplicateOrs(whereClause); - if (deDuplicatedWhere != null) { - whereClause = deDuplicatedWhere; - } } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java index 24ee4a33a0a154..a1d7c7b6b2fe57 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java @@ -33,6 +33,7 @@ import org.apache.doris.analysis.GroupingInfo; import org.apache.doris.analysis.InPredicate; import org.apache.doris.analysis.InlineViewRef; +import org.apache.doris.analysis.IntLiteral; import org.apache.doris.analysis.IsNullPredicate; import org.apache.doris.analysis.JoinOperator; import org.apache.doris.analysis.LiteralExpr; @@ -52,6 +53,7 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.FunctionSet; import org.apache.doris.catalog.MysqlTable; +import org.apache.doris.catalog.OlapTable; import org.apache.doris.catalog.Table; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.FeConstants; @@ -1360,7 +1362,15 @@ private PlanNode createScanNode(Analyzer analyzer, TableRef tblRef, SelectStmt s switch (tblRef.getTable().getType()) { case OLAP: - OlapScanNode olapNode = new OlapScanNode(ctx_.getNextNodeId(), tblRef.getDesc(), "OlapScanNode"); + OlapScanNode olapNode = new OlapScanNode(ctx_.getNextNodeId(), tblRef.getDesc(), + "OlapScanNode"); + if (((OlapTable) tblRef.getTable()).hasDeleteSign()) { + Expr conjunct = new BinaryPredicate(BinaryPredicate.Operator.EQ, + new SlotRef(tblRef.getAliasAsName(), Column.DELETE_SIGN), new IntLiteral(0)); + conjunct.analyze(analyzer); + analyzer.registerConjunct(conjunct, tblRef.getDesc().getId()); + } + olapNode.setForceOpenPreAgg(tblRef.isForcePreAggOpened()); scanNode = olapNode; break; diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java index ab7c96ebf5760b..74e736a8ec88bf 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java @@ -429,15 +429,13 @@ public void testVarcharToLongSupport() throws Exception { } @Test public void testDeleteSign() throws Exception { - ConnectContext ctx = UtFrameUtils.createDefaultCtx(); String sql1 = "SELECT * FROM db1.table1 LEFT ANTI JOIN db1.table2 ON db1.table1.siteid = db1.table2.siteid;"; - SelectStmt stmt1 = (SelectStmt) UtFrameUtils.parseAndAnalyzeStmt(sql1, ctx); - Assert.assertTrue(stmt1.toSql().contains("ON (`default_cluster:db1`.`table2`.`__DORIS_DELETE_SIGN__` = 0) " + - "AND ((`default_cluster:db1`.`table1`.`__DORIS_DELETE_SIGN__` = 0) AND " + - "(`db1`.`table1`.`siteid` = `db1`.`table2`.`siteid`))")); + Assert.assertTrue(dorisAssert.query(sql1).explainQuery().contains("`table1`.`__DORIS_DELETE_SIGN__` = 0")); String sql2 = "SELECT * FROM db1.table1 JOIN db1.table2 ON db1.table1.siteid = db1.table2.siteid;"; - SelectStmt stmt2 = (SelectStmt) UtFrameUtils.parseAndAnalyzeStmt(sql2, ctx); - Assert.assertTrue(stmt2.toSql().contains(" WHERE (`default_cluster:db1`.`table2`.`__DORIS_DELETE_SIGN__` = 0)" + - " AND (`default_cluster:db1`.`table1`.`__DORIS_DELETE_SIGN__` = 0)")); + Assert.assertTrue(dorisAssert.query(sql2).explainQuery().contains("`table1`.`__DORIS_DELETE_SIGN__` = 0")); + String sql3 = "SELECT * FROM table1"; + Assert.assertTrue(dorisAssert.query(sql3).explainQuery().contains("`table1`.`__DORIS_DELETE_SIGN__` = 0")); + String sql4 = " SELECT * FROM table1 table2"; + Assert.assertTrue(dorisAssert.query(sql4).explainQuery().contains("`table2`.`__DORIS_DELETE_SIGN__` = 0")); } } From 1a2df8fbd8d935abdb803e3c355924bb7e51cbd0 Mon Sep 17 00:00:00 2001 From: yangzhg Date: Tue, 1 Sep 2020 19:57:09 +0800 Subject: [PATCH 3/6] move filter to scanNode --- .../main/java/org/apache/doris/analysis/BaseTableRef.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java index 7dbce382171b53..3eceea0fbb3949 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseTableRef.java @@ -52,11 +52,6 @@ public TableRef clone() { return new BaseTableRef(this); } - @Override - public Table getTable() { - return table; - } - @Override public TupleDescriptor createTupleDescriptor(Analyzer analyzer) { TupleDescriptor result = analyzer.getDescTbl().createTupleDescriptor(); From 6e9cd93f2e487b75def37a51ef59c43972be4b2d Mon Sep 17 00:00:00 2001 From: yangzhg Date: Tue, 1 Sep 2020 19:58:25 +0800 Subject: [PATCH 4/6] move filter to scanNode --- .../main/java/org/apache/doris/planner/SingleNodePlanner.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java index a1d7c7b6b2fe57..81964fdbf47cc8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java @@ -1362,8 +1362,7 @@ private PlanNode createScanNode(Analyzer analyzer, TableRef tblRef, SelectStmt s switch (tblRef.getTable().getType()) { case OLAP: - OlapScanNode olapNode = new OlapScanNode(ctx_.getNextNodeId(), tblRef.getDesc(), - "OlapScanNode"); + OlapScanNode olapNode = new OlapScanNode(ctx_.getNextNodeId(), tblRef.getDesc(), "OlapScanNode"); if (((OlapTable) tblRef.getTable()).hasDeleteSign()) { Expr conjunct = new BinaryPredicate(BinaryPredicate.Operator.EQ, new SlotRef(tblRef.getAliasAsName(), Column.DELETE_SIGN), new IntLiteral(0)); From 05c6f795f632831c574e09bef034d5638c2c0a10 Mon Sep 17 00:00:00 2001 From: yangzhg Date: Wed, 2 Sep 2020 11:29:51 +0800 Subject: [PATCH 5/6] remove unused exceptions --- .../src/main/java/org/apache/doris/analysis/SelectStmt.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java index e69c0bac6bf32a..908649d3ff3f52 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java @@ -523,7 +523,7 @@ public List collectTupleIds() { return result; } - private void whereClauseRewrite() throws AnalysisException { + private void whereClauseRewrite() { if (whereClause instanceof IntLiteral) { if (((IntLiteral) whereClause).getLongValue() == 0) { whereClause = new BoolLiteral(false); From 740589185f82357c96b278cf7d922f8f285fcb15 Mon Sep 17 00:00:00 2001 From: yangzhg Date: Wed, 2 Sep 2020 16:04:10 +0800 Subject: [PATCH 6/6] fix bug --- .../src/main/java/org/apache/doris/analysis/SelectStmt.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java index 908649d3ff3f52..b9abe316d97141 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java @@ -531,6 +531,10 @@ private void whereClauseRewrite() { whereClause = new BoolLiteral(true); } } + Expr deDuplicatedWhere = deduplicateOrs(whereClause); + if (deDuplicatedWhere != null) { + whereClause = deDuplicatedWhere; + } } /**