From 6acee1ce886d533500438d24ec495f5bf86237dc Mon Sep 17 00:00:00 2001 From: lihangyu <15605149486@163.com> Date: Fri, 17 Feb 2023 10:59:35 +0800 Subject: [PATCH] [Fix](topn opt) double check plan From OriginalPlanner to make sure optimized SQL is a general topn query (#16848) From the original logic, query like `select * from a where exists (select * from b order by 1) order by 1 limit 1` is a query contains subquery, but the top query will pass `checkEnableTwoPhaseRead` and set `isTwoPhaseOptEnabled=true`.So check the double plan is a general topn query plan is needed, and rollback the needMaterialize flag setted by the previous `analyze`. --- .../org/apache/doris/analysis/DescriptorTable.java | 4 ++++ .../java/org/apache/doris/analysis/SelectStmt.java | 2 +- .../org/apache/doris/analysis/SlotDescriptor.java | 4 ++-- .../main/java/org/apache/doris/analysis/SlotRef.java | 4 ++-- .../org/apache/doris/planner/OriginalPlanner.java | 12 +++++++++++- .../data/query_p0/subquery/test_subquery.out | 12 ++++++++++++ .../suites/query_p0/subquery/test_subquery.groovy | 4 ++++ 7 files changed, 36 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorTable.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorTable.java index 7c1b62dcc5..72c4803439 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorTable.java @@ -118,6 +118,10 @@ public class DescriptorTable { return tupleDescs.get(id); } + public HashMap getSlotDescs() { + return slotDescs; + } + /** * Return all tuple desc by idList. */ 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 09fee4da91..f06d0385c8 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 @@ -660,7 +660,7 @@ public class SelectStmt extends QueryStmt { // reset slots need to do fetch column for (SlotRef slot : resultSlots) { // invalid slots will be pruned from reading from ScanNode - slot.setInvalid(); + slot.setNeedMaterialize(false); } LOG.debug("resultsSlots {}", resultSlots); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java index 0713ff0009..0fb28f829b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java @@ -111,8 +111,8 @@ public class SlotDescriptor { return isAgg; } - public void setInvalid() { - this.needMaterialize = false; + public void setNeedMaterialize(boolean needMaterialize) { + this.needMaterialize = needMaterialize; } public boolean isInvalid() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java index c1a2c5b14c..5d5b700454 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java @@ -124,8 +124,8 @@ public class SlotRef extends Expr { return desc.getId(); } - public void setInvalid() { - this.desc.setInvalid(); + public void setNeedMaterialize(boolean needMaterialize) { + this.desc.setNeedMaterialize(needMaterialize); } public boolean isInvalid() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java b/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java index f03c9fe1fa..85b8b7e339 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java @@ -283,7 +283,17 @@ public class OriginalPlanner extends Planner { } } else if (selectStmt.isTwoPhaseReadOptEnabled()) { // Optimize query like `SELECT ... FROM WHERE ... ORDER BY ... LIMIT ...` - injectRowIdColumnSlot(); + if (singleNodePlan instanceof SortNode + && singleNodePlan.getChildren().size() == 1 + && ((SortNode) singleNodePlan).getChild(0) instanceof OlapScanNode) { + // Double check this plan to ensure it's a general topn query + injectRowIdColumnSlot(); + } else { + // This is not a general topn query, rollback needMaterialize flag + for (SlotDescriptor slot : analyzer.getDescTbl().getSlotDescs().values()) { + slot.setNeedMaterialize(true); + } + } } } } diff --git a/regression-test/data/query_p0/subquery/test_subquery.out b/regression-test/data/query_p0/subquery/test_subquery.out index b9941ebf23..c77a083ae5 100644 --- a/regression-test/data/query_p0/subquery/test_subquery.out +++ b/regression-test/data/query_p0/subquery/test_subquery.out @@ -15,3 +15,15 @@ -- !sql4 -- 1 +-- !sql5 -- +\N \N \N \N \N \N \N \N \N \N \N \N \N \N +false 1 1989 1001 11011902 123.123 true 1989-03-21 1989-03-21T13:00 wangjuoo4 0.1 6.333 string12345 170141183460469231731687303715884105727 +false 2 1986 1001 11011903 1243.500 false 1901-12-31 1989-03-21T13:00 wangynnsf 20.268 789.25 string12345 -170141183460469231731687303715884105727 +false 3 1989 1002 11011905 24453.325 false 2012-03-14 2000-01-01T00:00 yunlj8@nk 78945.0 3654.0 string12345 0 +false 4 1991 3021 -11011907 243243.325 false 3124-10-10 2015-03-13T10:30 yanvjldjlll 2.06 -0.001 string12345 20220101 +false 5 1985 5014 -11011903 243.325 true 2015-01-01 2015-03-13T12:36:38 du3lnvl -0.0 -365.0 string12345 20220102 +false 6 32767 3021 123456 604587.000 true 2014-11-11 2015-03-13T12:36:38 yanavnd 0.1 80699.0 string12345 20220104 +false 7 -32767 1002 7210457 3.141 false 1988-03-21 1901-01-01T00:00 jiw3n4 0.0 6058.0 string12345 -20220101 +true 8 255 2147483647 11011920 -0.123 true 1989-03-21 9999-11-11T12:12 wangjuoo5 987456.123 12.14 string12345 -2022 +true 9 1991 -2147483647 11011902 -654.654 true 1991-08-11 1989-03-21T13:11 wangjuoo4 0.0 69.123 string12345 11011903 + diff --git a/regression-test/suites/query_p0/subquery/test_subquery.groovy b/regression-test/suites/query_p0/subquery/test_subquery.groovy index 1044259929..992476ab72 100644 --- a/regression-test/suites/query_p0/subquery/test_subquery.groovy +++ b/regression-test/suites/query_p0/subquery/test_subquery.groovy @@ -41,4 +41,8 @@ suite("test_subquery") { select /*+SET_VAR(enable_projection=false) */ count() from (select k2, k1 from test_query_db.baseall order by k1 limit 1) a; """ + + qt_sql5 """ + select * from test_query_db.bigtable where exists (select k2, k1 from test_query_db.baseall order by k1) order by k1, k2, k3, k4 limit 10; + """ }