You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2023/01/05 06:29:21 UTC
[GitHub] [doris] eldenmoon opened a new pull request, #15642: [Enhancement](topn) support two phase read for topn query
eldenmoon opened a new pull request, #15642:
URL: https://github.com/apache/doris/pull/15642
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
1. Does it affect the original behavior:
- [ ] Yes
- [ ] No
- [ ] I don't know
2. Has unit tests been added:
- [ ] Yes
- [ ] No
- [ ] No Need
3. Has document been added or modified:
- [ ] Yes
- [ ] No
- [ ] No Need
4. Does it need to update dependencies:
- [ ] Yes
- [ ] No
5. Are there any changes that cannot be rolled back:
- [ ] Yes (If Yes, please explain WHY)
- [ ] No
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1384791418
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] eldenmoon commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by 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
[GitHub] [doris] eldenmoon commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
eldenmoon commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071095007
##########
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 code, to be removed
##########
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
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1385340114
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071057702
##########
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:
move this line to line 53 and remove line 56 and 64
##########
be/src/vec/exec/vexchange_node.cpp:
##########
@@ -45,10 +48,14 @@ Status VExchangeNode::init(const TPlanNode& tnode, RuntimeState* state) {
if (!_is_merging) {
return Status::OK();
}
-
RETURN_IF_ERROR(_vsort_exec_exprs.init(tnode.exchange_node.sort_info, _pool));
_is_asc_order = tnode.exchange_node.sort_info.is_asc_order;
_nulls_first = tnode.exchange_node.sort_info.nulls_first;
+
+ if (tnode.exchange_node.__isset.nodes_info) {
+ _nodes_info = _pool->add(new DorisNodesInfo(tnode.exchange_node.nodes_info));
+ }
+ _use_two_phase_read = tnode.exchange_node.sort_info.use_two_phase_read;
Review Comment:
```suggestion
_use_two_phase_read = tnode.exchange_node.sort_info.__isset.use_two_phase_read && tnode.exchange_node.sort_info.use_two_phase_read;
```
##########
be/src/vec/exec/vexchange_node.cpp:
##########
@@ -122,6 +157,7 @@ Status VExchangeNode::get_next(RuntimeState* state, Block* block, bool* eos) {
}
COUNTER_SET(_rows_returned_counter, _num_rows_returned);
}
+ RETURN_IF_ERROR(second_phase_fetch_data(state, block));
Review Comment:
I think `status` should be check before calling `second_phase_fetch_data`.
##########
be/src/vec/exec/vexchange_node.cpp:
##########
@@ -89,6 +96,28 @@ Status VExchangeNode::open(RuntimeState* state) {
return Status::OK();
}
+Status VExchangeNode::second_phase_fetch_data(RuntimeState* state, Block* final_block) {
+ if (!_use_two_phase_read) {
+ return Status::OK();
+ }
+ if (final_block->rows() == 0) {
+ return Status::OK();
+ }
+ auto row_id_col = final_block->get_by_position(final_block->columns() - 1);
+ MonotonicStopWatch watch;
+ watch.start();
+ auto tuple_desc = _row_descriptor.tuple_descriptors()[0];
+ RowIDFetcher id_fetcher(tuple_desc, state);
+ RETURN_IF_ERROR(id_fetcher.init(_nodes_info));
+ MutableBlock materialized_block(_row_descriptor.tuple_descriptors(), final_block->rows());
+ // fetch will sort block by sequence of ROWID_COL
+ RETURN_IF_ERROR(id_fetcher.fetch(row_id_col.column, &materialized_block));
+ // Notice swap may change the structure of final_block
+ final_block->swap(materialized_block.to_block());
+ LOG(INFO) << "fetch_id finished, cost(ms):" << watch.elapsed_time() / 1000 / 1000;
Review Comment:
Use a profiler instead
##########
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:
add some comments for `is_invalid`. To be honest, I think this is a bit confusing. How about to rename it to `should_ignore` or `need_materialize`?
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1384296703
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1073339625
##########
be/src/olap/rowset/segment_v2/column_reader.h:
##########
@@ -417,6 +417,60 @@ class ArrayFileColumnIterator final : public ColumnIterator {
vectorized::ColumnArray::ColumnOffsets& column_offsets);
};
+class RowIdColumnIterator : public ColumnIterator {
+public:
+ RowIdColumnIterator() = delete;
+ RowIdColumnIterator(int32_t tid, RowsetId rid, int32_t segid)
+ : _tablet_id(tid), _rowset_id(rid), _segment_id(segid) {}
+
+ Status seek_to_first() override {
+ _current_rowid = 0;
+ return Status::OK();
+ }
+
+ Status seek_to_ordinal(ordinal_t ord_idx) override {
+ _current_rowid = ord_idx;
+ return Status::OK();
+ }
+
+ Status next_batch(size_t* n, vectorized::MutableColumnPtr& dst) {
+ bool has_null;
+ return next_batch(n, dst, &has_null);
+ }
+
+ Status next_batch(size_t* n, ColumnBlockView* dst, bool* has_null) override {
Review Comment:
warning: unknown type name 'ColumnBlockView' [clang-diagnostic-error]
```cpp
Status next_batch(size_t* n, ColumnBlockView* dst, bool* has_null) override {
^
```
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1385036290
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] eldenmoon commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
eldenmoon commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071094772
##########
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 uinque id to read in the second phase fetch
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1069232765
##########
be/src/vec/exec/vsort_node.h:
##########
@@ -76,6 +76,7 @@ class VSortNode final : public doris::ExecNode {
Field old_top {Field::Types::Null};
bool _reuse_mem;
+ bool _use_two_phase_read = false;
Review Comment:
warning: private field '_use_two_phase_read' is not used [clang-diagnostic-unused-private-field]
```cpp
bool _use_two_phase_read = false;
^
```
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071080244
##########
fe/fe-core/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -1947,5 +1947,8 @@ public class Config extends ConfigBase {
*/
@ConfField(masterOnly = true)
public static int hms_events_polling_interval_ms = 20000;
+
+ @ConfField(mutable = false)
+ public static int topn_two_phase_limit_threshold = 1024;
Review Comment:
change this to a session variable
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1384315670
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1386435052
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071100028
##########
be/src/exec/rowid_fetcher.cpp:
##########
@@ -0,0 +1,131 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "exec/rowid_fetcher.h"
+
+#include "bthread/countdown_event.h"
+#include "exec/tablet_info.h" // DorisNodesInfo
+#include "gen_cpp/Types_types.h"
+#include "gen_cpp/internal_service.pb.h"
+#include "runtime/exec_env.h" // ExecEnv
+#include "runtime/runtime_state.h" // RuntimeState
+#include "util/brpc_client_cache.h" // BrpcClientCache
+#include "util/defer_op.h"
+#include "vec/core/block.h" // Block
+
+namespace doris {
+
+Status RowIDFetcher::init(DorisNodesInfo* nodes_info) {
+ for (auto [node_id, node_info] : nodes_info->nodes_info()) {
+ auto client = ExecEnv::GetInstance()->brpc_internal_client_cache()->get_client(
+ node_info.host, node_info.brpc_port);
+ if (!client) {
+ LOG(WARNING) << "Get rpc stub failed, host=" << node_info.host
+ << ", port=" << node_info.brpc_port;
+ return Status::InternalError("RowIDFetcher failed to init rpc client");
+ }
+ _stubs.push_back(client);
+ }
+ return Status::OK();
+}
+
+static std::string format_rowid(const GlobalRowLoacation& location) {
+ return fmt::format("{} {} {} {}", location.tablet_id,
+ location.row_location.rowset_id.to_string(),
+ location.row_location.segment_id, location.row_location.row_id);
+}
+
+PMultiGetRequest RowIDFetcher::init_fetch_request(const vectorized::ColumnString& row_ids) {
+ PMultiGetRequest mget_req;
+ _tuple_desc->to_protobuf(mget_req.mutable_desc());
+ for (auto slot : _tuple_desc->slots()) {
+ slot->to_protobuf(mget_req.add_slots());
+ }
+ for (size_t i = 0; i < row_ids.size(); ++i) {
+ PMultiGetRequest::RowId row_id;
+ StringRef row_id_rep = row_ids.get_data_at(i);
+ auto location = reinterpret_cast<const GlobalRowLoacation*>(row_id_rep.data);
+ row_id.set_tablet_id(location->tablet_id);
+ row_id.set_rowset_id(location->row_location.rowset_id.to_string());
+ row_id.set_segment_id(location->row_location.segment_id);
+ row_id.set_ordinal_id(location->row_location.row_id);
+ *mget_req.add_rowids() = std::move(row_id);
+ }
+ mget_req.set_be_exec_version(_st->be_exec_version());
+ return mget_req;
+}
+
+static void fetch_callback(bthread::CountdownEvent* counter) {
+ Defer __defer([&] { counter->signal(); });
+}
+
+static Status MergeRPCResults(const std::vector<PMultiGetResponse>& rsps,
+ const std::vector<brpc::Controller>& cntls,
+ vectorized::MutableBlock* output_block) {
+ for (const auto& cntl : cntls) {
+ if (cntl.Failed()) {
+ LOG(WARNING) << "Failed to fetch meet rpc error:" << cntl.ErrorText()
+ << ", host:" << cntl.remote_side();
+ return Status::InternalError(cntl.ErrorText());
+ }
+ }
+ for (const auto& resp : rsps) {
+ Status st(resp.status());
+ if (!st.ok()) {
+ LOG(WARNING) << "Failed to fetch " << st.to_string();
+ return st;
+ }
+ vectorized::Block partial_block(resp.block());
+ output_block->merge(partial_block);
+ }
+ return Status::OK();
+}
+
+Status RowIDFetcher::fetch(const vectorized::ColumnPtr& row_ids,
+ vectorized::MutableBlock* res_block) {
+ assert(!_stubs.empty());
+ res_block->clear_column_data();
+ vectorized::MutableBlock mblock({_tuple_desc}, row_ids->size());
+ PMultiGetRequest mget_req = init_fetch_request(assert_cast<const vectorized::ColumnString&>(
+ *vectorized::remove_nullable(row_ids).get()));
Review Comment:
Why is `row_ids` nullable?
--
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
[GitHub] [doris] eldenmoon commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
eldenmoon commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071216360
##########
be/src/exec/rowid_fetcher.cpp:
##########
@@ -0,0 +1,131 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "exec/rowid_fetcher.h"
+
+#include "bthread/countdown_event.h"
+#include "exec/tablet_info.h" // DorisNodesInfo
+#include "gen_cpp/Types_types.h"
+#include "gen_cpp/internal_service.pb.h"
+#include "runtime/exec_env.h" // ExecEnv
+#include "runtime/runtime_state.h" // RuntimeState
+#include "util/brpc_client_cache.h" // BrpcClientCache
+#include "util/defer_op.h"
+#include "vec/core/block.h" // Block
+
+namespace doris {
+
+Status RowIDFetcher::init(DorisNodesInfo* nodes_info) {
+ for (auto [node_id, node_info] : nodes_info->nodes_info()) {
+ auto client = ExecEnv::GetInstance()->brpc_internal_client_cache()->get_client(
+ node_info.host, node_info.brpc_port);
+ if (!client) {
+ LOG(WARNING) << "Get rpc stub failed, host=" << node_info.host
+ << ", port=" << node_info.brpc_port;
+ return Status::InternalError("RowIDFetcher failed to init rpc client");
+ }
+ _stubs.push_back(client);
+ }
+ return Status::OK();
+}
+
+static std::string format_rowid(const GlobalRowLoacation& location) {
+ return fmt::format("{} {} {} {}", location.tablet_id,
+ location.row_location.rowset_id.to_string(),
+ location.row_location.segment_id, location.row_location.row_id);
+}
+
+PMultiGetRequest RowIDFetcher::init_fetch_request(const vectorized::ColumnString& row_ids) {
+ PMultiGetRequest mget_req;
+ _tuple_desc->to_protobuf(mget_req.mutable_desc());
+ for (auto slot : _tuple_desc->slots()) {
+ slot->to_protobuf(mget_req.add_slots());
+ }
+ for (size_t i = 0; i < row_ids.size(); ++i) {
+ PMultiGetRequest::RowId row_id;
+ StringRef row_id_rep = row_ids.get_data_at(i);
+ auto location = reinterpret_cast<const GlobalRowLoacation*>(row_id_rep.data);
+ row_id.set_tablet_id(location->tablet_id);
+ row_id.set_rowset_id(location->row_location.rowset_id.to_string());
+ row_id.set_segment_id(location->row_location.segment_id);
+ row_id.set_ordinal_id(location->row_location.row_id);
+ *mget_req.add_rowids() = std::move(row_id);
+ }
+ mget_req.set_be_exec_version(_st->be_exec_version());
+ return mget_req;
+}
+
+static void fetch_callback(bthread::CountdownEvent* counter) {
+ Defer __defer([&] { counter->signal(); });
+}
+
+static Status MergeRPCResults(const std::vector<PMultiGetResponse>& rsps,
+ const std::vector<brpc::Controller>& cntls,
+ vectorized::MutableBlock* output_block) {
+ for (const auto& cntl : cntls) {
+ if (cntl.Failed()) {
+ LOG(WARNING) << "Failed to fetch meet rpc error:" << cntl.ErrorText()
+ << ", host:" << cntl.remote_side();
+ return Status::InternalError(cntl.ErrorText());
+ }
+ }
+ for (const auto& resp : rsps) {
+ Status st(resp.status());
+ if (!st.ok()) {
+ LOG(WARNING) << "Failed to fetch " << st.to_string();
+ return st;
+ }
+ vectorized::Block partial_block(resp.block());
+ output_block->merge(partial_block);
+ }
+ return Status::OK();
+}
+
+Status RowIDFetcher::fetch(const vectorized::ColumnPtr& row_ids,
+ vectorized::MutableBlock* res_block) {
+ assert(!_stubs.empty());
Review Comment:
done
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071081437
##########
be/src/exprs/runtime_filter.h:
##########
@@ -31,7 +31,7 @@ namespace doris {
class Predicate;
class ObjectPool;
class ExprContext;
-class RuntimeState;
+// class RuntimeState;
Review Comment:
```suggestion
```
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071099127
##########
be/src/exec/rowid_fetcher.cpp:
##########
@@ -0,0 +1,131 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "exec/rowid_fetcher.h"
+
+#include "bthread/countdown_event.h"
+#include "exec/tablet_info.h" // DorisNodesInfo
+#include "gen_cpp/Types_types.h"
+#include "gen_cpp/internal_service.pb.h"
+#include "runtime/exec_env.h" // ExecEnv
+#include "runtime/runtime_state.h" // RuntimeState
+#include "util/brpc_client_cache.h" // BrpcClientCache
+#include "util/defer_op.h"
+#include "vec/core/block.h" // Block
+
+namespace doris {
+
+Status RowIDFetcher::init(DorisNodesInfo* nodes_info) {
+ for (auto [node_id, node_info] : nodes_info->nodes_info()) {
+ auto client = ExecEnv::GetInstance()->brpc_internal_client_cache()->get_client(
+ node_info.host, node_info.brpc_port);
+ if (!client) {
+ LOG(WARNING) << "Get rpc stub failed, host=" << node_info.host
+ << ", port=" << node_info.brpc_port;
+ return Status::InternalError("RowIDFetcher failed to init rpc client");
+ }
+ _stubs.push_back(client);
+ }
+ return Status::OK();
+}
+
+static std::string format_rowid(const GlobalRowLoacation& location) {
+ return fmt::format("{} {} {} {}", location.tablet_id,
+ location.row_location.rowset_id.to_string(),
+ location.row_location.segment_id, location.row_location.row_id);
+}
+
+PMultiGetRequest RowIDFetcher::init_fetch_request(const vectorized::ColumnString& row_ids) {
+ PMultiGetRequest mget_req;
+ _tuple_desc->to_protobuf(mget_req.mutable_desc());
+ for (auto slot : _tuple_desc->slots()) {
+ slot->to_protobuf(mget_req.add_slots());
+ }
+ for (size_t i = 0; i < row_ids.size(); ++i) {
+ PMultiGetRequest::RowId row_id;
+ StringRef row_id_rep = row_ids.get_data_at(i);
+ auto location = reinterpret_cast<const GlobalRowLoacation*>(row_id_rep.data);
+ row_id.set_tablet_id(location->tablet_id);
+ row_id.set_rowset_id(location->row_location.rowset_id.to_string());
+ row_id.set_segment_id(location->row_location.segment_id);
+ row_id.set_ordinal_id(location->row_location.row_id);
+ *mget_req.add_rowids() = std::move(row_id);
+ }
+ mget_req.set_be_exec_version(_st->be_exec_version());
+ return mget_req;
+}
+
+static void fetch_callback(bthread::CountdownEvent* counter) {
+ Defer __defer([&] { counter->signal(); });
+}
+
+static Status MergeRPCResults(const std::vector<PMultiGetResponse>& rsps,
+ const std::vector<brpc::Controller>& cntls,
+ vectorized::MutableBlock* output_block) {
+ for (const auto& cntl : cntls) {
+ if (cntl.Failed()) {
+ LOG(WARNING) << "Failed to fetch meet rpc error:" << cntl.ErrorText()
+ << ", host:" << cntl.remote_side();
+ return Status::InternalError(cntl.ErrorText());
+ }
+ }
+ for (const auto& resp : rsps) {
+ Status st(resp.status());
+ if (!st.ok()) {
+ LOG(WARNING) << "Failed to fetch " << st.to_string();
+ return st;
+ }
+ vectorized::Block partial_block(resp.block());
+ output_block->merge(partial_block);
+ }
+ return Status::OK();
+}
+
+Status RowIDFetcher::fetch(const vectorized::ColumnPtr& row_ids,
+ vectorized::MutableBlock* res_block) {
+ assert(!_stubs.empty());
Review Comment:
```suggestion
CHECK(!_stubs.empty());
```
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1384603262
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1387222219
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] hello-stephen commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1372528425
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 33.4 seconds
load time: 480 seconds
storage size: 17122353329 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230105173942_clickbench_pr_74465.html
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1383492452
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071033878
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java:
##########
@@ -591,6 +591,33 @@ public void analyze(Analyzer analyzer) throws UserException {
analyzeAggregation(analyzer);
createAnalyticInfo(analyzer);
eliminatingSortNode();
+ if (checkEnableTwoPhaseRead()) {
+ // If optimize enabled, we try our best to read less columns from ScanNode,
+ // here we analyze conjunct exprs and ordering exprs before resultExprs,
+ // 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
+ LOG.info("two phase read optimize enabled");
Review Comment:
```suggestion
LOG.debug("two phase read optimize enabled");
```
##########
gensrc/thrift/PlanNodes.thrift:
##########
@@ -891,6 +894,8 @@ struct TExchangeNode {
2: optional TSortInfo sort_info
// This is tHe number of rows to skip before returning results
3: optional i64 offset
+ // used for second phase fetch
Review Comment:
```suggestion
// Nodes in this cluster, used for second phase fetch
```
##########
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:
Why is light schema change required?
##########
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:
Is this always false?
##########
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() {
Review Comment:
Add a session variable to turn off this feature
##########
fe/fe-core/src/main/java/org/apache/doris/planner/SortNode.java:
##########
@@ -142,6 +144,15 @@ public void setUseTopnOpt(boolean useTopnOpt) {
this.useTopnOpt = useTopnOpt;
}
+ public List<Expr> getResolvedTupleExprs() {
+ return resolvedTupleExprs;
+ }
+
+ public boolean isUseTopNTwoPhaseOptimize() {
Review Comment:
```suggestion
public boolean useTwoPhaseTopNOptimization() {
```
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1381801570
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15642:
URL: https://github.com/apache/doris/pull/15642#issuecomment-1386903548
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] Gabriel39 merged pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 merged PR #15642:
URL: https://github.com/apache/doris/pull/15642
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1062156640
##########
be/src/vec/exec/scan/new_olap_scan_node.cpp:
##########
@@ -179,13 +264,46 @@ static std::string tablets_id_to_string(
return ss.str();
}
+// iterate through conjuncts tree
+void NewOlapScanNode::_iterate_conjuncts_tree(const VExpr* conjunct_expr_root,
+ std::function<void(const VExpr*)> fn) {
+ if (!conjunct_expr_root) {
+ return;
+ }
+ fn(conjunct_expr_root);
+ for (const VExpr* child : conjunct_expr_root->children()) {
+ _iterate_conjuncts_tree(child, fn);
+ }
+ return;
+}
Review Comment:
warning: redundant return statement at the end of a function with a void return type [readability-redundant-control-flow]
```suggestion
}
```
##########
be/src/vec/exec/scan/new_olap_scan_node.cpp:
##########
@@ -179,13 +264,46 @@
return ss.str();
}
+// iterate through conjuncts tree
+void NewOlapScanNode::_iterate_conjuncts_tree(const VExpr* conjunct_expr_root,
+ std::function<void(const VExpr*)> fn) {
+ if (!conjunct_expr_root) {
+ return;
+ }
+ fn(conjunct_expr_root);
+ for (const VExpr* child : conjunct_expr_root->children()) {
+ _iterate_conjuncts_tree(child, fn);
+ }
+ return;
+}
+
+// get all slot ref column unique ids
+void NewOlapScanNode::_collect_conjuncts_slot_column_unique_ids(const VExpr* expr) {
+ if (!expr->is_slot_ref()) {
+ return;
+ }
+ auto slot_ref = reinterpret_cast<const VSlotRef*>(expr);
+ for (const auto* slot_desc : _output_tuple_desc->slots()) {
+ if (slot_desc->id() == slot_ref->slot_id() && slot_desc->col_unique_id() > 0) {
+ _conjuct_column_unique_ids.emplace(slot_desc->col_unique_id());
+ }
+ }
+ return;
+}
Review Comment:
warning: redundant return statement at the end of a function with a void return type [readability-redundant-control-flow]
```suggestion
}
```
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071095580
##########
be/src/vec/exec/vexchange_node.h:
##########
@@ -47,6 +49,9 @@ class VExchangeNode : public ExecNode {
// Status collect_query_statistics(QueryStatistics* statistics) override;
void set_num_senders(int num_senders) { _num_senders = num_senders; }
+ // final materializtion, used only in topn node
+ Status second_phase_fetch_data(RuntimeState* state, Block* final_block);
Review Comment:
```suggestion
Status _second_phase_fetch_data(RuntimeState* state, Block* final_block);
```
--
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
[GitHub] [doris] Gabriel39 commented on a diff in pull request #15642: [Enhancement](topn) support two phase read for topn query
Posted by GitBox <gi...@apache.org>.
Gabriel39 commented on code in PR #15642:
URL: https://github.com/apache/doris/pull/15642#discussion_r1071112128
##########
be/src/service/internal_service.cpp:
##########
@@ -949,4 +956,131 @@ void PInternalServiceImpl::response_slave_tablet_pull_rowset(
Status::OK().to_protobuf(response->mutable_status());
}
+static Status read_by_rowids(
+ std::pair<size_t, size_t> row_range_idx, const TupleDescriptor& desc,
+ const google::protobuf::RepeatedPtrField<PMultiGetRequest_RowId>& rowids,
+ vectorized::Block* sub_block) {
+ //read from row_range.first to row_range.second
+ for (size_t i = row_range_idx.first; i < row_range_idx.second; ++i) {
+ MonotonicStopWatch watch;
+ watch.start();
+ auto row_id = rowids[i];
+ TabletSharedPtr tablet = StorageEngine::instance()->tablet_manager()->get_tablet(
+ row_id.tablet_id(), true /*include deleted*/);
+ RowsetId rowset_id;
+ rowset_id.init(row_id.rowset_id());
+ if (!tablet) {
+ continue;
+ }
+ BetaRowsetSharedPtr rowset =
+ std::static_pointer_cast<BetaRowset>(tablet->get_rowset(rowset_id));
+ if (!rowset) {
+ LOG(INFO) << "no such rowset " << rowset_id;
+ continue;
+ }
+ const TabletSchemaSPtr tablet_schema = rowset->tablet_schema();
+ // const TabletSchemaSPtr tablet_schema = tablet->tablet_schema();
Review Comment:
delete this line
--
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