From 85521944dd453864d9409b2c748d5e0db8ccd6ea Mon Sep 17 00:00:00 2001 From: zhoubintao <35688959+zbtzbtzbt@users.noreply.github.com> Date: Thu, 16 Dec 2021 10:33:41 +0800 Subject: [PATCH] [refactor](olap-scan-node) Refactor olap scannode (#7131) 1. Delete useless variables 2. Add const modifier for read-only function 3. Delete the empty destructor, the compiler will automatically generate it, refer to the 3/5/0 rule: [https://en.cppreference.com/w/cpp/language/rule_of_three] 4. It is recommended to add the override keyword (instead of the virtual keyword) to the subclass virtual function. Override will let the compiler help check and improve security. This is also the reason why C++11 introduces override --- be/src/exec/exec_node.cpp | 4 ++++ be/src/exec/exec_node.h | 8 ++++---- be/src/exec/olap_scan_node.cpp | 3 --- be/src/exec/olap_scan_node.h | 23 +++++++++++------------ be/src/exec/olap_scanner.cpp | 1 - be/src/exec/olap_scanner.h | 2 -- be/src/exec/scan_node.h | 5 ++--- be/src/runtime/fragment_mgr.cpp | 3 --- be/src/runtime/plan_fragment_executor.cpp | 2 +- be/src/runtime/plan_fragment_executor.h | 2 -- 10 files changed, 22 insertions(+), 31 deletions(-) diff --git a/be/src/exec/exec_node.cpp b/be/src/exec/exec_node.cpp index 8d0c8bfc93..cdd4917c51 100644 --- a/be/src/exec/exec_node.cpp +++ b/be/src/exec/exec_node.cpp @@ -656,6 +656,10 @@ Status ExecNode::QueryMaintenance(RuntimeState* state, const std::string& msg) { return state->check_query_state(msg); } +Status ExecNode::get_next(RuntimeState* state, RowBatch* row_batch, bool* eos) { + return Status::NotSupported("Not Implemented get batch"); +} + Status ExecNode::get_next(RuntimeState* state, vectorized::Block* block, bool* eos) { return Status::NotSupported("Not Implemented get block"); } diff --git a/be/src/exec/exec_node.h b/be/src/exec/exec_node.h index 63f048e252..c58f862b72 100644 --- a/be/src/exec/exec_node.h +++ b/be/src/exec/exec_node.h @@ -100,7 +100,7 @@ public: // row_batch's tuple_data_pool. // Caller must not be holding any io buffers. This will cause deadlock. // TODO: AggregationNode and HashJoinNode cannot be "re-opened" yet. - virtual Status get_next(RuntimeState* state, RowBatch* row_batch, bool* eos) = 0; + virtual Status get_next(RuntimeState* state, RowBatch* row_batch, bool* eos); virtual Status get_next(RuntimeState* state, vectorized::Block* block, bool* eos); // Resets the stream of row batches to be retrieved by subsequent GetNext() calls. @@ -184,17 +184,17 @@ public: const RowDescriptor& row_desc() const { return _row_descriptor; } int64_t rows_returned() const { return _num_rows_returned; } int64_t limit() const { return _limit; } - bool reached_limit() { return _limit != -1 && _num_rows_returned >= _limit; } + bool reached_limit() const { return _limit != -1 && _num_rows_returned >= _limit; } const std::vector& get_tuple_ids() const { return _tuple_ids; } - RuntimeProfile* runtime_profile() { return _runtime_profile.get(); } + RuntimeProfile* runtime_profile() const { return _runtime_profile.get(); } RuntimeProfile::Counter* memory_used_counter() const { return _memory_used_counter; } std::shared_ptr mem_tracker() const { return _mem_tracker; } std::shared_ptr expr_mem_tracker() const { return _expr_mem_tracker; } - MemPool* expr_mem_pool() { return _expr_mem_pool.get(); } + MemPool* expr_mem_pool() const { return _expr_mem_pool.get(); } // Extract node id from p->name(). static int get_node_id_from_profile(RuntimeProfile* p); diff --git a/be/src/exec/olap_scan_node.cpp b/be/src/exec/olap_scan_node.cpp index 1cb610df34..5e4ba33867 100644 --- a/be/src/exec/olap_scan_node.cpp +++ b/be/src/exec/olap_scan_node.cpp @@ -59,8 +59,6 @@ OlapScanNode::OlapScanNode(ObjectPool* pool, const TPlanNode& tnode, const Descr _eval_conjuncts_fn(nullptr), _runtime_filter_descs(tnode.runtime_filters) {} -OlapScanNode::~OlapScanNode() {} - Status OlapScanNode::init(const TPlanNode& tnode, RuntimeState* state) { RETURN_IF_ERROR(ExecNode::init(tnode, state)); _direct_conjunct_size = state->enable_vectorized_exec() ? 1 : _conjunct_ctxs.size(); @@ -1671,6 +1669,5 @@ Status OlapScanNode::add_one_batch(RowBatch* row_batch) { return Status::OK(); } -void OlapScanNode::debug_string(int /* indentation_level */, std::stringstream* /* out */) const {} } // namespace doris diff --git a/be/src/exec/olap_scan_node.h b/be/src/exec/olap_scan_node.h index 983f0515f0..3b7df03452 100644 --- a/be/src/exec/olap_scan_node.h +++ b/be/src/exec/olap_scan_node.h @@ -49,21 +49,20 @@ enum TransferStatus { class OlapScanNode : public ScanNode { public: OlapScanNode(ObjectPool* pool, const TPlanNode& tnode, const DescriptorTbl& descs); - ~OlapScanNode(); - virtual Status init(const TPlanNode& tnode, RuntimeState* state = nullptr); - virtual Status prepare(RuntimeState* state); - virtual Status open(RuntimeState* state); - virtual Status get_next(RuntimeState* state, RowBatch* row_batch, bool* eos); + Status init(const TPlanNode& tnode, RuntimeState* state = nullptr) override; + Status prepare(RuntimeState* state) override; + Status open(RuntimeState* state) override; + Status get_next(RuntimeState* state, RowBatch* row_batch, bool* eos) override; Status collect_query_statistics(QueryStatistics* statistics) override; - virtual Status close(RuntimeState* state); - virtual Status set_scan_ranges(const std::vector& scan_ranges); + Status close(RuntimeState* state) override; + Status set_scan_ranges(const std::vector & scan_ranges) override; inline void set_no_agg_finalize() { _need_agg_finalize = false; } protected: - typedef struct { + struct HeapType { Tuple* tuple; int id; - } HeapType; + }; class MergeComparison { public: @@ -82,7 +81,7 @@ protected: typedef std::priority_queue, MergeComparison> Heap; - void display_heap(Heap& heap) { + void display_heap(const Heap& heap) const { Heap h = heap; std::stringstream s; s << "Heap: ["; @@ -134,9 +133,9 @@ protected: Status add_one_batch(RowBatch* row_batch); // Write debug string of this into out. - virtual void debug_string(int indentation_level, std::stringstream* out) const; + void debug_string(int indentation_level, std::stringstream* out) const override {} - const std::vector& runtime_filter_descs() { return _runtime_filter_descs; } + const std::vector& runtime_filter_descs() const { return _runtime_filter_descs; } void _init_counter(RuntimeState* state); // OLAP_SCAN_NODE profile layering: OLAP_SCAN_NODE, OlapScanner, and SegmentIterator diff --git a/be/src/exec/olap_scanner.cpp b/be/src/exec/olap_scanner.cpp index f250f255fd..2e3af6be50 100644 --- a/be/src/exec/olap_scanner.cpp +++ b/be/src/exec/olap_scanner.cpp @@ -61,7 +61,6 @@ OlapScanner::OlapScanner(RuntimeState* runtime_state, OlapScanNode* parent, bool _rows_pushed_cond_filtered_counter = parent->_rows_pushed_cond_filtered_counter; } -OlapScanner::~OlapScanner() {} Status OlapScanner::prepare( const TPaloScanRange& scan_range, const std::vector& key_ranges, diff --git a/be/src/exec/olap_scanner.h b/be/src/exec/olap_scanner.h index 1e86dcef2d..899cb4dedb 100644 --- a/be/src/exec/olap_scanner.h +++ b/be/src/exec/olap_scanner.h @@ -51,8 +51,6 @@ public: OlapScanner(RuntimeState* runtime_state, OlapScanNode* parent, bool aggregation, bool need_agg_finalize, const TPaloScanRange& scan_range); - ~OlapScanner(); - Status prepare(const TPaloScanRange& scan_range, const std::vector& key_ranges, const std::vector& filters, const std::vector>>& diff --git a/be/src/exec/scan_node.h b/be/src/exec/scan_node.h index 2ce04226ba..5f8ec4b082 100644 --- a/be/src/exec/scan_node.h +++ b/be/src/exec/scan_node.h @@ -69,16 +69,15 @@ class ScanNode : public ExecNode { public: ScanNode(ObjectPool* pool, const TPlanNode& tnode, const DescriptorTbl& descs) : ExecNode(pool, tnode, descs) {} - virtual ~ScanNode() {} // Set up counters - virtual Status prepare(RuntimeState* state); + Status prepare(RuntimeState* state) override; // Convert scan_ranges into node-specific scan restrictions. This should be // called after prepare() virtual Status set_scan_ranges(const std::vector& scan_ranges) = 0; - virtual bool is_scan_node() const { return true; } + bool is_scan_node() const override { return true; } RuntimeProfile::Counter* bytes_read_counter() const { return _bytes_read_counter; } RuntimeProfile::Counter* rows_read_counter() const { return _rows_read_counter; } diff --git a/be/src/runtime/fragment_mgr.cpp b/be/src/runtime/fragment_mgr.cpp index dab2046750..0946732ac1 100644 --- a/be/src/runtime/fragment_mgr.cpp +++ b/be/src/runtime/fragment_mgr.cpp @@ -82,7 +82,6 @@ public: FragmentExecState(const TUniqueId& query_id, const TUniqueId& instance_id, int backend_num, ExecEnv* exec_env, const TNetworkAddress& coord_addr); - ~FragmentExecState(); Status prepare(const TExecPlanFragmentParams& params); @@ -166,7 +165,6 @@ private: int _timeout_second; - std::unique_ptr _exec_thread; // This context is shared by all fragments of this host in a query std::shared_ptr _fragments_ctx; @@ -209,7 +207,6 @@ FragmentExecState::FragmentExecState(const TUniqueId& query_id, _start_time = DateTimeValue::local_time(); } -FragmentExecState::~FragmentExecState() {} Status FragmentExecState::prepare(const TExecPlanFragmentParams& params) { if (params.__isset.query_options) { diff --git a/be/src/runtime/plan_fragment_executor.cpp b/be/src/runtime/plan_fragment_executor.cpp index a2ac3a3903..a464211fdb 100644 --- a/be/src/runtime/plan_fragment_executor.cpp +++ b/be/src/runtime/plan_fragment_executor.cpp @@ -587,4 +587,4 @@ void PlanFragmentExecutor::close() { _closed = true; } -} // namespace doris +} // namespace doris \ No newline at end of file diff --git a/be/src/runtime/plan_fragment_executor.h b/be/src/runtime/plan_fragment_executor.h index 6280ab85e9..1fc4578240 100644 --- a/be/src/runtime/plan_fragment_executor.h +++ b/be/src/runtime/plan_fragment_executor.h @@ -207,8 +207,6 @@ private: std::shared_ptr _query_statistics; bool _collect_query_statistics_with_every_batch; - bool _enable_vectorized_engine; - ObjectPool* obj_pool() { return _runtime_state->obj_pool(); } // typedef for TPlanFragmentExecParams.per_node_scan_ranges