github-actions[bot] commented on code in PR #27557:
URL: https://github.com/apache/doris/pull/27557#discussion_r1404241868
##########
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_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/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_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/exec/join/process_hash_table_probe_impl.h:
##########
@@ -412,17 +196,18 @@ 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,
+
hash_table_ctx.hash_table->get_visited(),
+
hash_table_ctx.hash_table->has_null_key());
}
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 83
(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:212:** +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:221:** +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:221:** +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:236:** +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:243:** +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:247:** +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:249:** +1, nesting
level increased to 3
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:252:** +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:257:** +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: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:261:** +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:267:** +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:273:** +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:278:** +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:280:** +4, including
nesting penalty of 3, nesting level increased to 4
```cpp
if (has_null_in_build_side &&
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:283:** +1, nesting
level increased to 4
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:287:** +1, nesting
level increased to 2
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:288:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
if constexpr (JoinOpType == TJoinOp::LEFT_SEMI_JOIN) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:289:** +4, including
nesting penalty of 3, nesting level increased to 4
```cpp
for (size_t i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:290:** +5, including
nesting penalty of 4, nesting level increased to 5
```cpp
if (filter_column_ptr[i]) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:293:** +1, nesting
level increased to 5
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:297:** +1, nesting
level increased to 3
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:298:** +4, including
nesting penalty of 3, nesting level increased to 4
```cpp
for (size_t i = 0; i < row_count; ++i) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:299:** +5, including
nesting penalty of 4, nesting level increased to 5
```cpp
if (_build_indexs[i]) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:301:** +6, including
nesting penalty of 5, nesting level increased to 6
```cpp
if (filter_column_ptr[i]) {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:304:** +1, nesting
level increased to 5
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:312:** +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:314:** +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:317:** +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:319:** +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:326:** +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:329:** +1, nesting
level increased to 1
```cpp
} else {
^
```
**be/src/vec/exec/join/process_hash_table_probe_impl.h:330:** +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:335:** +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:336:** +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:335:** +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.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/vec/common/hash_table/hash_map.h:
##########
@@ -20,9 +20,14 @@
#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_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/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,
```
##########
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(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/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) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
