github-actions[bot] commented on code in PR #27397:
URL: https://github.com/apache/doris/pull/27397#discussion_r1401551830
##########
be/src/exprs/minmax_predicate.h:
##########
@@ -50,32 +55,78 @@ class MinMaxNumFunc : public MinMaxFuncBase {
T val_data = *reinterpret_cast<const T*>(data);
- if (_empty) {
- _min = val_data;
- _max = val_data;
- _empty = false;
- return;
+ if constexpr (NeedMin) {
+ if (val_data < _min) {
+ _min = val_data;
+ }
}
- if (val_data < _min) {
- _min = val_data;
- } else if (val_data > _max) {
- _max = val_data;
+
+ if constexpr (NeedMax) {
+ if (val_data > _max) {
+ _max = val_data;
+ }
}
}
- void insert_fixed_len(const char* data, const int* offsets, int number)
override {
- if (!number) {
+ void insert_fixed_len(const vectorized::ColumnPtr& column, size_t start)
override {
Review Comment:
warning: function 'insert_fixed_len' has cognitive complexity of 65
(threshold 50) [readability-function-cognitive-complexity]
```cpp
void insert_fixed_len(const vectorized::ColumnPtr& column, size_t start)
override {
^
```
<details>
<summary>Additional context</summary>
**be/src/exprs/minmax_predicate.h:71:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (column->empty()) {
^
```
**be/src/exprs/minmax_predicate.h:74:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (column->is_nullable()) {
^
```
**be/src/exprs/minmax_predicate.h:81:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if constexpr (std::is_same_v<T, StringRef>) {
^
```
**be/src/exprs/minmax_predicate.h:83:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
for (size_t i = start; i < column->size(); i++) {
^
```
**be/src/exprs/minmax_predicate.h:84:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (!nullmap[i]) {
^
```
**be/src/exprs/minmax_predicate.h:85:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
if constexpr (NeedMin) {
^
```
**be/src/exprs/minmax_predicate.h:88:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
if constexpr (NeedMax) {
^
```
**be/src/exprs/minmax_predicate.h:93:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/exprs/minmax_predicate.h:95:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
for (size_t i = start; i < column->size(); i++) {
^
```
**be/src/exprs/minmax_predicate.h:96:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (!nullmap[i]) {
^
```
**be/src/exprs/minmax_predicate.h:97:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
if constexpr (NeedMin) {
^
```
**be/src/exprs/minmax_predicate.h:100:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
if constexpr (NeedMax) {
^
```
**be/src/exprs/minmax_predicate.h:106:** +1, nesting level increased to 1
```cpp
} else {
^
```
**be/src/exprs/minmax_predicate.h:107:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if constexpr (std::is_same_v<T, StringRef>) {
^
```
**be/src/exprs/minmax_predicate.h:109:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
for (size_t i = start; i < column->size(); i++) {
^
```
**be/src/exprs/minmax_predicate.h:110:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if constexpr (NeedMin) {
^
```
**be/src/exprs/minmax_predicate.h:113:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if constexpr (NeedMax) {
^
```
**be/src/exprs/minmax_predicate.h:117:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/exprs/minmax_predicate.h:119:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
for (size_t i = start; i < column->size(); i++) {
^
```
**be/src/exprs/minmax_predicate.h:120:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if constexpr (NeedMin) {
^
```
**be/src/exprs/minmax_predicate.h:123:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if constexpr (NeedMax) {
^
```
</details>
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -508,24 +476,33 @@ class RuntimePredicateWrapper {
}
}
- void insert_batch(const vectorized::ColumnPtr column, const
std::vector<int>& rows) {
+ void insert_batch(const vectorized::ColumnPtr& column, size_t start) {
if (get_real_type() == RuntimeFilterType::BITMAP_FILTER) {
- bitmap_filter_insert_batch(column, rows);
- } else if (IRuntimeFilter::enable_use_batch(_be_exec_version > 0,
_column_return_type)) {
- insert_fixed_len(column->get_raw_data().data, rows.data(),
rows.size());
+ bitmap_filter_insert_batch(column, start);
} else {
- for (int index : rows) {
- insert(column->get_data_at(index));
- }
+ insert_fixed_len(column, start);
}
}
- void bitmap_filter_insert_batch(const vectorized::ColumnPtr column,
- const std::vector<int>& rows) {
+ void bitmap_filter_insert_batch(const vectorized::ColumnPtr column, size_t
start) {
Review Comment:
warning: method 'bitmap_filter_insert_batch' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void bitmap_filter_insert_batch(const vectorized::ColumnPtr
column, size_t start) {
```
##########
be/src/exprs/runtime_filter_slots.h:
##########
@@ -37,7 +37,7 @@
const std::vector<TRuntimeFilterDesc>& runtime_filter_descs)
: _build_expr_context(build_expr_ctxs),
_runtime_filter_descs(runtime_filter_descs) {}
- Status init(RuntimeState* state, int64_t hash_table_size, size_t
build_bf_cardinality) {
+ Status init(RuntimeState* state, int64_t hash_table_size) {
Review Comment:
warning: function 'init' exceeds recommended size/complexity thresholds
[readability-function-size]
```cpp
Status init(RuntimeState* state, int64_t hash_table_size) {
^
```
<details>
<summary>Additional context</summary>
**be/src/exprs/runtime_filter_slots.h:39:** 123 lines including whitespace
and comments (threshold 80)
```cpp
Status init(RuntimeState* state, int64_t hash_table_size) {
^
```
</details>
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -161,6 +161,43 @@ void ColumnString::insert_indices_from(const IColumn& src,
const int* indices_be
}
}
+void ColumnString::insert_indices_from_join(const IColumn& src, const
uint32_t* indices_begin,
Review Comment:
warning: method 'insert_indices_from_join' can be made static
[readability-convert-member-functions-to-static]
be/src/vec/columns/column_string.h:489:
```diff
- void insert_indices_from_join(const IColumn& src, const uint32_t*
indices_begin,
+ static void insert_indices_from_join(const IColumn& src, const
uint32_t* indices_begin,
```
##########
be/src/vec/exec/join/process_hash_table_probe_impl.h:
##########
@@ -412,17 +199,17 @@ Status ProcessHashTableProbe<JoinOpType,
Parent>::do_process(HashTableType& hash
output_block->swap(mutable_block.to_block());
if constexpr (with_other_conjuncts) {
- return do_other_join_conjuncts(output_block, is_mark_join,
multi_matched_output_row_count,
- is_the_last_sub_block);
+ return do_other_join_conjuncts(output_block, is_mark_join,
is_the_last_sub_block,
+
hash_table_ctx.hash_table->get_visited());
}
return Status::OK();
}
template <int JoinOpType, typename Parent>
Status ProcessHashTableProbe<JoinOpType, Parent>::do_other_join_conjuncts(
Review Comment:
warning: function 'do_other_join_conjuncts' has cognitive complexity of 58
(threshold 50) [readability-function-cognitive-complexity]
```cpp
Status ProcessHashTableProbe<JoinOpType, Parent>::do_other_join_conjuncts(
^
```
<details>
<summary>Additional context</summary>
**be/src/vec/exec/join/process_hash_table_probe_impl.h:214:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if (!row_count) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:223:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
RETURN_IF_ERROR(VExprContext::execute_conjuncts(_parent->_other_join_conjuncts,
nullptr,
^
```
**be/src/common/status.h:523:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:223:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(VExprContext::execute_conjuncts(_parent->_other_join_conjuncts,
nullptr,
^
```
**be/src/common/status.h:525:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:238:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if constexpr (JoinOpType == TJoinOp::LEFT_OUTER_JOIN ||
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:247:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (int i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:251:** +1
```cpp
null_map_data[i] = !join_hit || !other_hit;
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:253:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
if (!join_hit) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:255:** +1, nesting
level increased to 3
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:258:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
if (filter_map[i]) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:263:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (size_t i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:264:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
if (filter_map[i]) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:266:** +4, including
nesting penalty of 3, nesting level increased to 4
```cpp
if constexpr (JoinOpType == TJoinOp::FULL_OUTER_JOIN) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:272:** +1, nesting
level increased to 1
```cpp
} else if constexpr (JoinOpType == TJoinOp::LEFT_SEMI_JOIN) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:276:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (size_t i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:281:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
if (is_mark_join) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:288:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
for (size_t i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:294:** +1, nesting
level increased to 1
```cpp
} else if constexpr (JoinOpType == TJoinOp::LEFT_ANTI_JOIN ||
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:308:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (size_t i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:312:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
if (is_mark_join) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:317:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
for (int i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:323:** +1, nesting
level increased to 1
```cpp
} else if constexpr (JoinOpType == TJoinOp::RIGHT_SEMI_JOIN ||
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:325:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (int i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:328:** +1, nesting
level increased to 1
```cpp
} else if constexpr (JoinOpType == TJoinOp::RIGHT_OUTER_JOIN) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:330:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
for (int i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:337:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if constexpr (JoinOpType == TJoinOp::RIGHT_SEMI_JOIN ||
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:340:** +1, nesting
level increased to 1
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:341:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
if constexpr (JoinOpType == TJoinOp::LEFT_SEMI_JOIN ||
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:346:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(Block::filter_block(output_block, result_column_id,
^
```
**be/src/common/status.h:523:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:347:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
is_mark_join ?
output_block->columns() : orig_columns));
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:346:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(Block::filter_block(output_block, result_column_id,
^
```
**be/src/common/status.h:525:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
</details>
##########
be/src/exprs/runtime_filter_slots.h:
##########
@@ -37,7 +37,7 @@ class VRuntimeFilterSlots {
const std::vector<TRuntimeFilterDesc>& runtime_filter_descs)
: _build_expr_context(build_expr_ctxs),
_runtime_filter_descs(runtime_filter_descs) {}
- Status init(RuntimeState* state, int64_t hash_table_size, size_t
build_bf_cardinality) {
+ Status init(RuntimeState* state, int64_t hash_table_size) {
Review Comment:
warning: method 'init' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status init(RuntimeState* state, int64_t hash_table_size) {
```
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -161,6 +161,43 @@
}
}
+void ColumnString::insert_indices_from_join(const IColumn& src, const
uint32_t* indices_begin,
+ const uint32_t* indices_end) {
+ const ColumnString& src_str = assert_cast<const ColumnString&>(src);
+ auto src_offset_data = src_str.offsets.data();
+
+ auto old_char_size = chars.size();
+ size_t total_chars_size = old_char_size;
+
+ auto dst_offsets_pos = offsets.size();
+ offsets.resize(offsets.size() + indices_end - indices_begin);
+ auto* dst_offsets_data = offsets.data();
+
+ for (auto x = indices_begin; x != indices_end; ++x) {
+ if (*x != 0) {
+ total_chars_size += src_offset_data[*x] - src_offset_data[*x - 1];
+ }
+ dst_offsets_data[dst_offsets_pos++] = total_chars_size;
+ }
+ check_chars_length(total_chars_size, offsets.size());
+
+ chars.resize(total_chars_size);
+
+ auto* src_data_ptr = src_str.chars.data();
+ auto* dst_data_ptr = chars.data();
+
+ size_t dst_chars_pos = old_char_size;
+ for (auto x = indices_begin; x != indices_end; ++x) {
Review Comment:
warning: 'auto x' can be declared as 'const auto *x'
[readability-qualified-auto]
```suggestion
for (const auto *x = indices_begin; x != indices_end; ++x) {
```
##########
be/src/vec/columns/column_map.cpp:
##########
@@ -196,6 +196,17 @@ void ColumnMap::insert_indices_from(const IColumn& src,
const int* indices_begin
}
}
+void ColumnMap::insert_indices_from_join(const IColumn& src, const uint32_t*
indices_begin,
+ const uint32_t* indices_end) {
+ for (auto x = indices_begin; x != indices_end; ++x) {
Review Comment:
warning: 'auto x' can be declared as 'const auto *x'
[readability-qualified-auto]
```suggestion
for (const auto *x = indices_begin; x != indices_end; ++x) {
```
##########
be/src/vec/columns/column_string.cpp:
##########
@@ -161,6 +161,43 @@
}
}
+void ColumnString::insert_indices_from_join(const IColumn& src, const
uint32_t* indices_begin,
+ const uint32_t* indices_end) {
+ const ColumnString& src_str = assert_cast<const ColumnString&>(src);
+ auto src_offset_data = src_str.offsets.data();
+
+ auto old_char_size = chars.size();
+ size_t total_chars_size = old_char_size;
+
+ auto dst_offsets_pos = offsets.size();
+ offsets.resize(offsets.size() + indices_end - indices_begin);
+ auto* dst_offsets_data = offsets.data();
+
+ for (auto x = indices_begin; x != indices_end; ++x) {
Review Comment:
warning: 'auto x' can be declared as 'const auto *x'
[readability-qualified-auto]
```suggestion
for (const auto *x = indices_begin; x != indices_end; ++x) {
```
##########
be/src/vec/columns/column_array.cpp:
##########
@@ -808,6 +808,17 @@ void ColumnArray::insert_indices_from(const IColumn& src,
const int* indices_beg
}
}
+void ColumnArray::insert_indices_from_join(const IColumn& src, const uint32_t*
indices_begin,
+ const uint32_t* indices_end) {
+ for (auto x = indices_begin; x != indices_end; ++x) {
Review Comment:
warning: 'auto x' can be declared as 'const auto *x'
[readability-qualified-auto]
```suggestion
for (const auto *x = indices_begin; x != indices_end; ++x) {
```
##########
be/src/vec/common/hash_table/hash_map.h:
##########
@@ -20,9 +20,16 @@
#pragma once
+#include <gen_cpp/PlanNodes_types.h>
Review Comment:
warning: 'gen_cpp/PlanNodes_types.h' file not found [clang-diagnostic-error]
```cpp
#include <gen_cpp/PlanNodes_types.h>
^
```
##########
be/src/vec/columns/column_struct.cpp:
##########
@@ -233,6 +233,15 @@ void ColumnStruct::insert_indices_from(const IColumn& src,
const int* indices_be
}
}
+void ColumnStruct::insert_indices_from_join(const IColumn& src, const
uint32_t* indices_begin,
Review Comment:
warning: method 'insert_indices_from_join' can be made static
[readability-convert-member-functions-to-static]
be/src/vec/columns/column_struct.h:126:
```diff
- void insert_indices_from_join(const IColumn& src, const uint32_t*
indices_begin,
+ static void insert_indices_from_join(const IColumn& src, const
uint32_t* indices_begin,
```
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
