This is an automated email from the ASF dual-hosted git repository.
gabriellee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 1d0fdff98a [Bug](sort) disable 2phase read for sort by expressions
exclude slotref (#16460)
1d0fdff98a is described below
commit 1d0fdff98a2a31e113059ae98b056b5add036cd5
Author: lihangyu <15605149...@163.com>
AuthorDate: Tue Feb 7 19:42:54 2023 +0800
[Bug](sort) disable 2phase read for sort by expressions exclude slotref
(#16460)
```
create table tbl1 (k1 varchar(100), k2 string) distributed by hash(k1)
buckets 1 properties("replication_num" = "1");
insert into tbl1 values(1, "alice");
select cast(k1 as INT) as id from tbl1 order by id limit 2;
```
The above query could pass `checkEnableTwoPhaseRead` since the order by
element is SlotRef but actually it's an function call expr
---
.../java/org/apache/doris/analysis/SelectStmt.java | 18 ++++++++++--
.../org/apache/doris/planner/OriginalPlanner.java | 4 +--
regression-test/data/query_p0/sort/sort.out | 32 ++++++++++++++++++++++
regression-test/suites/query_p0/sort/sort.groovy | 13 +++++++++
4 files changed, 63 insertions(+), 4 deletions(-)
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 034717f886..bfb6286b43 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
@@ -128,6 +128,8 @@ public class SelectStmt extends QueryStmt {
// For quick get condition for point query
private Map<SlotRef, Expr> eqPredicates;
+ boolean isTwoPhaseOptEnabled = false;
+
public SelectStmt(ValueList valueList, ArrayList<OrderByElement>
orderByElement, LimitElement limitElement) {
super(orderByElement, limitElement);
this.valueList = valueList;
@@ -603,6 +605,7 @@ public class SelectStmt extends QueryStmt {
// rest of resultExprs will be marked as `INVALID`, such columns
will
// be prevent from reading from ScanNode.Those columns will be
finally
// read by the second fetch phase
+ isTwoPhaseOptEnabled = true;
LOG.debug("two phase read optimize enabled");
// Expr.analyze(resultExprs, analyzer);
Set<SlotRef> resultSlots = Sets.newHashSet();
@@ -620,10 +623,12 @@ public class SelectStmt extends QueryStmt {
// invalid slots will be pruned from reading from ScanNode
slot.setInvalid();
}
+
LOG.debug("resultsSlots {}", resultSlots);
LOG.debug("orderingSlots {}", orderingSlots);
LOG.debug("conjuntSlots {}", conjuntSlots);
}
+ checkAndSetPointQuery();
if (evaluateOrderBy) {
createSortTupleInfo(analyzer);
}
@@ -648,6 +653,10 @@ public class SelectStmt extends QueryStmt {
}
}
+ public boolean isTwoPhaseReadOptEnabled() {
+ return isTwoPhaseOptEnabled;
+ }
+
// Check whether enable two phase read optimize, if enabled query will be
devieded into two phase read:
// 1. read conjuncts columns and order by columns along with an extra
RowId column from ScanNode
// 2. sort and filter data, and get final RowId column, spawn RPC to other
BE to fetch final data
@@ -707,11 +716,12 @@ public class SelectStmt extends QueryStmt {
// Rethink? implement more generic to support all exprs
LOG.debug("getOrderingExprs {}", sortInfo.getOrderingExprs());
LOG.debug("getOrderByElements {}", getOrderByElements());
- for (OrderByElement orderby : getOrderByElements()) {
- if (!(orderby.getExpr() instanceof SlotRef)) {
+ for (Expr sortExpr : sortInfo.getOrderingExprs()) {
+ if (!(sortExpr instanceof SlotRef)) {
return false;
}
}
+ isTwoPhaseOptEnabled = true;
return true;
}
@@ -2376,6 +2386,10 @@ public class SelectStmt extends QueryStmt {
return eqPredicates;
}
+ public boolean isPointQueryShortCircuit() {
+ return isPointQuery;
+ }
+
// Check if it is a point query and set EQUAL predicates
public boolean checkAndSetPointQuery() {
if (isPointQuery) {
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 21d5b18dbe..5f0abd125d 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
@@ -263,7 +263,7 @@ public class OriginalPlanner extends Planner {
LOG.debug("this isn't block query");
}
// Check SelectStatement if optimization condition satisfied
- if (selectStmt.checkAndSetPointQuery()) {
+ if (selectStmt.isPointQueryShortCircuit()) {
// Optimize for point query like: SELECT * FROM t1 WHERE pk1 =
1 and pk2 = 2
// such query will use direct RPC to do point query
LOG.debug("it's a point query");
@@ -276,7 +276,7 @@ public class OriginalPlanner extends Planner {
analyzer.getPrepareStmt().cacheSerializedDescriptorTable(olapScanNode.getDescTable());
analyzer.getPrepareStmt().cacheSerializedOutputExprs(rootFragment.getOutputExprs());
}
- } else if (selectStmt.checkEnableTwoPhaseRead(analyzer)) {
+ } else if (selectStmt.isTwoPhaseReadOptEnabled()) {
// Optimize query like `SELECT ... FROM <tbl> WHERE ... ORDER
BY ... LIMIT ...`
injectRowIdColumnSlot();
}
diff --git a/regression-test/data/query_p0/sort/sort.out
b/regression-test/data/query_p0/sort/sort.out
index 3d778981cc..c13b777bb9 100644
--- a/regression-test/data/query_p0/sort/sort.out
+++ b/regression-test/data/query_p0/sort/sort.out
@@ -12,3 +12,35 @@ true
-- !sql --
+-- !sql --
+1
+2
+3
+4
+
+-- !sql --
+0
+0
+1
+1
+
+-- !sql --
+1
+2
+3
+4
+
+-- !sql --
+1
+2
+3
+4
+
+-- !sql --
+1
+2
+
+-- !sql --
+1
+2
+
diff --git a/regression-test/suites/query_p0/sort/sort.groovy
b/regression-test/suites/query_p0/sort/sort.groovy
index 025ec3f198..016ef37dce 100644
--- a/regression-test/suites/query_p0/sort/sort.groovy
+++ b/regression-test/suites/query_p0/sort/sort.groovy
@@ -41,4 +41,17 @@ suite("sort") {
LIMIT 110
OFFSET 130
"""
+
+ sql """drop table if exists tbl1"""
+ sql """create table tbl1 (k1 varchar(100), k2 string) distributed by
hash(k1) buckets 1 properties("replication_num" = "1");"""
+ sql """insert into tbl1 values(1, "alice");"""
+ sql """insert into tbl1 values(2, "bob");"""
+ sql """insert into tbl1 values(3, "mark");"""
+ sql """insert into tbl1 values(4, "thor");"""
+ qt_sql """select cast(k1 as INT) as id from tbl1 order by id;"""
+ qt_sql """select cast(k1 as INT) % 2 as id from tbl1 order by id;"""
+ qt_sql """select cast(k1 as BIGINT) as id from tbl1 order by id;"""
+ qt_sql """select cast(k1 as STRING) as id from tbl1 order by id;"""
+ qt_sql """select cast(k1 as INT) as id from tbl1 order by id limit 2"""
+ qt_sql """select cast(k1 as STRING) as id from tbl1 order by id limit 2"""
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
