[GitHub] [doris] eldenmoon commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query

[GitBox <gi...@apache.org>]

eldenmoon commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071090895


##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -1052,6 +1060,10 @@ protected void toThrift(TPlanNode msg) {
         if (pushDownAggNoGroupingOp != null) {
             msg.olap_scan_node.setPushDownAggTypeOpt(pushDownAggNoGroupingOp);
         }
+
+        if (orderingExprs != null) {

Review Comment:
   redundant codes , tobe removed



##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -1052,6 +1060,10 @@ protected void toThrift(TPlanNode msg) {
         if (pushDownAggNoGroupingOp != null) {
             msg.olap_scan_node.setPushDownAggTypeOpt(pushDownAggNoGroupingOp);
         }
+
+        if (orderingExprs != null) {

Review Comment:
   redundant



##########
gensrc/thrift/Descriptors.thrift:
##########
@@ -51,6 +51,8 @@ struct TSlotDescriptor {
   9: required i32 slotIdx
   10: required bool isMaterialized
   11: optional i32 col_unique_id = -1
+  12: optional bool is_key = false
+  13: optional bool is_invalid = false

Review Comment:
   good advice
   



##########
be/src/vec/exprs/vslot_ref.cpp:
##########
@@ -50,6 +50,12 @@ Status VSlotRef::prepare(doris::RuntimeState* state, const doris::RowDescriptor&
     if (slot_desc == nullptr) {
         return Status::InternalError("couldn't resolve slot descriptor {}", _slot_id);
     }
+    if (slot_desc->invalid()) {
+        // invalid slot should be ignored manually
+        _column_id = -1;
+        _column_name = &slot_desc->col_name();

Review Comment:
   OK



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java:
##########
@@ -619,6 +646,39 @@ public void analyze(Analyzer analyzer) throws UserException {
         }
     }
 
+    // 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
+    // 3. final matrialize all data
+    public boolean checkEnableTwoPhaseRead() {
+        // Only handle the simplest `SELECT ... FROM <tbl> WHERE ... ORDER BY ... LIMIT ...` query
+        if (getAggInfo() != null
+                || getHavingPred() != null
+                || getWithClause() != null) {
+            return false;
+        }
+        // single olap table
+        List<TableRef> tblRefs = getTableRefs();
+        if (tblRefs.size() != 1 || !(tblRefs.get(0) instanceof BaseTableRef)) {
+            return false;
+        }
+        TableRef tbl = tblRefs.get(0);
+        if (tbl.getTable().getType() != Table.TableType.OLAP) {
+            return false;
+        }
+        // need enable light schema change

Review Comment:
   needs column unique id to read data in the second phase



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org