From 8326cefcc6588150badbaf64bdfa0319236856f5 Mon Sep 17 00:00:00 2001 From: LiBinfeng <46676950+LiBinfeng-01@users.noreply.github.com> Date: Fri, 12 Apr 2024 15:06:02 +0800 Subject: [PATCH] [Fix](Nereids) fix leading hint should have all tables in one query block (#33517) --- .../doris/nereids/hint/LeadingHint.java | 59 +++++++++---------- .../nereids/rules/analysis/LeadingJoin.java | 2 +- .../data/nereids_p0/hint/fix_leading.out | 50 ++++++++++++++++ .../suites/nereids_p0/hint/fix_leading.groovy | 5 ++ 4 files changed, 85 insertions(+), 31 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java index 39c1eab1383f1e..a1b1af1609bf8d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/hint/LeadingHint.java @@ -17,7 +17,6 @@ package org.apache.doris.nereids.hint; -import org.apache.doris.catalog.TableIf; import org.apache.doris.common.Pair; import org.apache.doris.nereids.jobs.joinorder.hypergraph.bitmap.LongBitmap; import org.apache.doris.nereids.trees.expressions.ExprId; @@ -191,14 +190,14 @@ public RelationId findRelationIdAndTableName(String name) { return null; } - private boolean hasSameName() { + private Optional hasSameName() { Set tableSet = Sets.newHashSet(); for (String table : tablelist) { if (!tableSet.add(table)) { - return true; + return Optional.of(table); } } - return false; + return Optional.empty(); } public Map getExprIdToTableNameMap() { @@ -252,12 +251,14 @@ public Long getTotalBitmap() { /** * set total bitmap used in leading before we get into leading join */ - public void setTotalBitmap() { + public void setTotalBitmap(Set inputRelationSets) { Long totalBitmap = 0L; - if (hasSameName()) { + Optional duplicateTableName = hasSameName(); + if (duplicateTableName.isPresent()) { this.setStatus(HintStatus.SYNTAX_ERROR); - this.setErrorMessage("duplicated table"); + this.setErrorMessage("duplicated table:" + duplicateTableName.get()); } + Set existRelationSets = new HashSet<>(); for (int index = 0; index < getTablelist().size(); index++) { RelationId id = findRelationIdAndTableName(getTablelist().get(index)); if (id == null) { @@ -265,11 +266,32 @@ public void setTotalBitmap() { this.setErrorMessage("can not find table: " + getTablelist().get(index)); return; } + existRelationSets.add(id); totalBitmap = LongBitmap.set(totalBitmap, id.asInt()); } + if (getTablelist().size() < inputRelationSets.size()) { + Set missRelationIds = new HashSet<>(); + missRelationIds.addAll(inputRelationSets); + missRelationIds.removeAll(existRelationSets); + String missingTablenames = getMissingTableNames(missRelationIds); + this.setStatus(HintStatus.SYNTAX_ERROR); + this.setErrorMessage("leading should have all tables in query block, missing tables: " + missingTablenames); + } this.totalBitmap = totalBitmap; } + private String getMissingTableNames(Set missRelationIds) { + String missTableNames = ""; + for (RelationId id : missRelationIds) { + for (Pair pair : relationIdAndTableName) { + if (pair.first.equals(id)) { + missTableNames += pair.second + " "; + } + } + } + return missTableNames; + } + /** * try to get join constraint, if can not get, it means join is inner join, * @param joinTableBitmap table bitmap below this join @@ -565,27 +587,4 @@ private Long getBitmap(LogicalPlan root) { return null; } } - - /** - * get leading containing tables which means leading wants to combine tables into joins - * @return long value represent tables we included - */ - public Long getLeadingTableBitmap(List tables) { - Long totalBitmap = 0L; - if (hasSameName()) { - this.setStatus(HintStatus.SYNTAX_ERROR); - this.setErrorMessage("duplicated table"); - return totalBitmap; - } - for (int index = 0; index < getTablelist().size(); index++) { - RelationId id = findRelationIdAndTableName(getTablelist().get(index)); - if (id == null) { - this.setStatus(HintStatus.SYNTAX_ERROR); - this.setErrorMessage("can not find table: " + getTablelist().get(index)); - return totalBitmap; - } - totalBitmap = LongBitmap.set(totalBitmap, id.asInt()); - } - return totalBitmap; - } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/LeadingJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/LeadingJoin.java index 833201634664ea..4d90f5226b3b8c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/LeadingJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/LeadingJoin.java @@ -46,7 +46,7 @@ public List buildRules() { return ctx.root; } Hint leadingHint = ctx.cascadesContext.getHintMap().get("Leading"); - ((LeadingHint) leadingHint).setTotalBitmap(); + ((LeadingHint) leadingHint).setTotalBitmap(ctx.root.getInputRelations()); Long currentBitMap = LongBitmap.computeTableBitmap(ctx.root.getInputRelations()); if (((LeadingHint) leadingHint).getTotalBitmap().equals(currentBitMap) && leadingHint.isSuccess()) { diff --git a/regression-test/data/nereids_p0/hint/fix_leading.out b/regression-test/data/nereids_p0/hint/fix_leading.out index 95e8befe6d9c80..feb69f97783e17 100644 --- a/regression-test/data/nereids_p0/hint/fix_leading.out +++ b/regression-test/data/nereids_p0/hint/fix_leading.out @@ -226,3 +226,53 @@ SyntaxError: -- !select4_2 -- 1000 + +-- !select4_3 -- +PhysicalResultSink +--hashAgg[GLOBAL] +----hashAgg[LOCAL] +------PhysicalProject +--------NestedLoopJoin[RIGHT_OUTER_JOIN](t3.c3 > 500) +----------PhysicalDistribute[DistributionSpecGather] +------------PhysicalProject +--------------NestedLoopJoin[LEFT_OUTER_JOIN](t1.c1 < 200)(t1.c1 > 500) +----------------PhysicalProject +------------------PhysicalOlapScan[t1] +----------------PhysicalDistribute[DistributionSpecReplicated] +------------------PhysicalProject +--------------------filter((t2.c2 > 500)) +----------------------PhysicalOlapScan[t2] +----------PhysicalDistribute[DistributionSpecGather] +------------PhysicalProject +--------------PhysicalOlapScan[t3] + +Hint log: +Used: leading(t1 t2 t3 ) +UnUsed: +SyntaxError: + +-- !select5_1 -- +PhysicalResultSink +--hashAgg[GLOBAL] +----PhysicalDistribute[DistributionSpecGather] +------hashAgg[LOCAL] +--------PhysicalProject +----------NestedLoopJoin[LEFT_OUTER_JOIN](t3.c3 > 500) +------------PhysicalProject +--------------PhysicalOlapScan[t3] +------------PhysicalDistribute[DistributionSpecReplicated] +--------------PhysicalProject +----------------NestedLoopJoin[LEFT_OUTER_JOIN](t1.c1 > 500) +------------------PhysicalProject +--------------------filter((t1.c1 < 200)) +----------------------PhysicalOlapScan[t1] +------------------PhysicalDistribute[DistributionSpecReplicated] +--------------------PhysicalProject +----------------------filter((t2.c2 > 500)) +------------------------PhysicalOlapScan[t2] + +Hint log: +Used: +UnUsed: +SyntaxError: leading(t1 t2) Msg:leading should have all tables in query block, missing tables: t3 + diff --git a/regression-test/suites/nereids_p0/hint/fix_leading.groovy b/regression-test/suites/nereids_p0/hint/fix_leading.groovy index 583a21d8700ef1..a82db224adfad9 100644 --- a/regression-test/suites/nereids_p0/hint/fix_leading.groovy +++ b/regression-test/suites/nereids_p0/hint/fix_leading.groovy @@ -166,4 +166,9 @@ suite("fix_leading") { // check left right join result qt_select4_1 """select count(*) from t1 left join t2 on c1 > 500 and c2 >500 right join t3 on c3 > 500 and c1 < 200;""" qt_select4_2 """select /*+ leading(t1 t2 t3)*/ count(*) from t1 left join t2 on c1 > 500 and c2 >500 right join t3 on c3 > 500 and c1 < 200;""" + qt_select4_3 """explain shape plan select /*+ leading(t1 t2 t3)*/ count(*) from t1 left join t2 on c1 > 500 and c2 >500 right join t3 on c3 > 500 and c1 < 200;""" + + // check whether we have all tables + qt_select5_1 """explain shape plan select /*+ leading(t1 t2)*/ count(*) from t1 left join t2 on c1 > 500 and c2 >500 right join t3 on c3 > 500 and c1 < 200;""" + }