github-actions[bot] commented on code in PR #32180:
URL: https://github.com/apache/doris/pull/32180#discussion_r1531836746
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -736,53 +727,52 @@
// assign this filter by protobuf
Status assign(const PBloomFilter* bloom_filter,
butil::IOBufAsZeroCopyInputStream* data,
bool contain_null) {
- _is_bloomfilter = true;
// we won't use this class to insert or find any data
// so any type is ok
-
_context.bloom_filter_func.reset(create_bloom_filter(_column_return_type ==
INVALID_TYPE
- ?
PrimitiveType::TYPE_INT
- :
_column_return_type));
- RETURN_IF_ERROR(_context.bloom_filter_func->assign(data,
bloom_filter->filter_length(),
- contain_null));
+
_context->bloom_filter_func.reset(create_bloom_filter(_column_return_type ==
INVALID_TYPE
+ ?
PrimitiveType::TYPE_INT
+ :
_column_return_type));
+ RETURN_IF_ERROR(_context->bloom_filter_func->assign(data,
bloom_filter->filter_length(),
+ contain_null));
return Status::OK();
}
// used by shuffle runtime filter
// assign this filter by protobuf
Status assign(const PMinMaxFilter* minmax_filter, bool contain_null) {
PrimitiveType type = to_primitive_type(minmax_filter->column_type());
- _context.minmax_func.reset(create_minmax_filter(type));
+ _context->minmax_func.reset(create_minmax_filter(type));
if (contain_null) {
- _context.minmax_func->set_null_aware(true);
- _context.minmax_func->set_contain_null();
+ _context->minmax_func->set_null_aware(true);
+ _context->minmax_func->set_contain_null();
}
switch (type) {
case TYPE_BOOLEAN: {
bool min_val = minmax_filter->min_val().boolval();
bool max_val = minmax_filter->max_val().boolval();
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
}
case TYPE_TINYINT: {
int8_t min_val =
static_cast<int8_t>(minmax_filter->min_val().intval());
int8_t max_val =
static_cast<int8_t>(minmax_filter->max_val().intval());
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
}
case TYPE_SMALLINT: {
int16_t min_val =
static_cast<int16_t>(minmax_filter->min_val().intval());
int16_t max_val =
static_cast<int16_t>(minmax_filter->max_val().intval());
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the
type name [modernize-use-auto]
```suggestion
auto max_val =
static_cast<int16_t>(minmax_filter->max_val().intval());
```
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -434,30 +440,37 @@ class RuntimePredicateWrapper {
bitmaps.push_back(&(col->get_data()[i]));
}
}
- _context.bitmap_filter_func->insert_many(bitmaps);
+ _context->bitmap_filter_func->insert_many(bitmaps);
}
RuntimeFilterType get_real_type() const {
- auto real_filter_type = _filter_type;
- if (real_filter_type == RuntimeFilterType::IN_OR_BLOOM_FILTER) {
- real_filter_type = _is_bloomfilter ?
RuntimeFilterType::BLOOM_FILTER
- : RuntimeFilterType::IN_FILTER;
+ if (_filter_type == RuntimeFilterType::IN_OR_BLOOM_FILTER) {
+ if (_context->hybrid_set) {
+ return RuntimeFilterType::IN_FILTER;
+ }
+ return RuntimeFilterType::BLOOM_FILTER;
}
- return real_filter_type;
+ return _filter_type;
}
size_t get_bloom_filter_size() const {
- if (_is_bloomfilter) {
- return _context.bloom_filter_func->get_size();
- }
- return 0;
+ return _context->bloom_filter_func ?
_context->bloom_filter_func->get_size() : 0;
}
Status get_push_exprs(std::list<vectorized::VExprContextSPtr>& probe_ctxs,
std::vector<vectorized::VRuntimeFilterPtr>&
push_exprs,
const TExpr& probe_expr);
Status merge(const RuntimePredicateWrapper* wrapper) {
Review Comment:
warning: function 'merge' has cognitive complexity of 73 (threshold 50)
[readability-function-cognitive-complexity]
```cpp
Status merge(const RuntimePredicateWrapper* wrapper) {
^
```
<details>
<summary>Additional context</summary>
**be/src/exprs/runtime_filter.cpp:464:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (is_ignored()) {
^
```
**be/src/exprs/runtime_filter.cpp:468:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
if (wrapper->is_ignored()) {
^
```
**be/src/exprs/runtime_filter.cpp:474:** +1
```cpp
_filter_type == RuntimeFilterType::IN_OR_BLOOM_FILTER &&
^
```
**be/src/exprs/runtime_filter.cpp:479:** +1
```cpp
bool can_not_merge_other = _filter_type !=
RuntimeFilterType::IN_OR_BLOOM_FILTER &&
^
```
**be/src/exprs/runtime_filter.cpp:482:** +1
```cpp
CHECK(!can_not_merge_in_or_bloom && !can_not_merge_other)
^
```
**be/src/exprs/runtime_filter.cpp:487:** +1, including nesting penalty of 0,
nesting level increased to 1
```cpp
switch (_filter_type) {
^
```
**be/src/exprs/runtime_filter.cpp:491:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (_max_in_num >= 0 && _context->hybrid_set->size() >=
_max_in_num) {
^
```
**be/src/exprs/runtime_filter.cpp:501:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/exprs/runtime_filter.cpp:501:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/exprs/runtime_filter.cpp:506:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
RETURN_IF_ERROR(
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/exprs/runtime_filter.cpp:506:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
RETURN_IF_ERROR(
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/exprs/runtime_filter.cpp:514:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (other_filter_type == RuntimeFilterType::IN_OR_BLOOM_FILTER) {
^
```
**be/src/exprs/runtime_filter.cpp:518:** +2, including nesting penalty of 1,
nesting level increased to 2
```cpp
if (real_filter_type == RuntimeFilterType::IN_FILTER) {
^
```
**be/src/exprs/runtime_filter.cpp:519:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
if (other_filter_type == RuntimeFilterType::IN_FILTER) { //
in merge in
^
```
**be/src/exprs/runtime_filter.cpp:521:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
if (_max_in_num >= 0 && _context->hybrid_set->size() >=
_max_in_num) {
^
```
**be/src/exprs/runtime_filter.cpp:525:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
RETURN_IF_ERROR(change_to_bloom_filter(true));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/exprs/runtime_filter.cpp:525:** +6, including nesting penalty of 5,
nesting level increased to 6
```cpp
RETURN_IF_ERROR(change_to_bloom_filter(true));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/exprs/runtime_filter.cpp:527:** +1, nesting level increased to 3
```cpp
} else {
^
```
**be/src/exprs/runtime_filter.cpp:530:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
RETURN_IF_ERROR(change_to_bloom_filter(!_build_bf_exactly));
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/exprs/runtime_filter.cpp:530:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
RETURN_IF_ERROR(change_to_bloom_filter(!_build_bf_exactly));
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/exprs/runtime_filter.cpp:531:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
RETURN_IF_ERROR(_context->bloom_filter_func->merge(
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/exprs/runtime_filter.cpp:531:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
RETURN_IF_ERROR(_context->bloom_filter_func->merge(
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/exprs/runtime_filter.cpp:534:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/exprs/runtime_filter.cpp:535:** +3, including nesting penalty of 2,
nesting level increased to 3
```cpp
if (other_filter_type == RuntimeFilterType::IN_FILTER) { //
bloom filter merge in
^
```
**be/src/exprs/runtime_filter.cpp:538:** +1, nesting level increased to 3
```cpp
} else {
^
```
**be/src/exprs/runtime_filter.cpp:539:** +4, including nesting penalty of 3,
nesting level increased to 4
```cpp
RETURN_IF_ERROR(_context->bloom_filter_func->merge(
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/exprs/runtime_filter.cpp:539:** +5, including nesting penalty of 4,
nesting level increased to 5
```cpp
RETURN_IF_ERROR(_context->bloom_filter_func->merge(
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
</details>
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -885,65 +875,65 @@
return Status::InvalidArgument("not support!");
}
- HybridSetBase::IteratorBase* get_in_filter_iterator() { return
_context.hybrid_set->begin(); }
+ HybridSetBase::IteratorBase* get_in_filter_iterator() { return
_context->hybrid_set->begin(); }
void get_bloom_filter_desc(char** data, int* filter_length) {
- _context.bloom_filter_func->get_data(data, filter_length);
+ _context->bloom_filter_func->get_data(data, filter_length);
}
void get_minmax_filter_desc(void** min_data, void** max_data) {
- *min_data = _context.minmax_func->get_min();
- *max_data = _context.minmax_func->get_max();
+ *min_data = _context->minmax_func->get_min();
+ *max_data = _context->minmax_func->get_max();
}
PrimitiveType column_type() { return _column_return_type; }
- bool is_bloomfilter() const { return _is_bloomfilter; }
+ bool is_bloomfilter() const { return get_real_type() ==
RuntimeFilterType::BLOOM_FILTER; }
bool contain_null() const {
- if (_is_bloomfilter) {
- return _context.bloom_filter_func->contain_null();
+ if (is_bloomfilter()) {
+ return _context->bloom_filter_func->contain_null();
}
- if (_context.hybrid_set) {
+ if (_context->hybrid_set) {
DCHECK(get_real_type() == RuntimeFilterType::IN_FILTER);
- return _context.hybrid_set->contain_null();
+ return _context->hybrid_set->contain_null();
}
- if (_context.minmax_func) {
- return _context.minmax_func->contain_null();
+ if (_context->minmax_func) {
+ return _context->minmax_func->contain_null();
}
return false;
}
- bool is_ignored() const { return _ignored; }
-
- const std::string& ignored_msg() const { return _ignored_msg; }
+ bool is_ignored() const { return _context->ignored; }
void batch_assign(const PInFilter* filter,
void (*assign_func)(std::shared_ptr<HybridSetBase>&
_hybrid_set,
PColumnValue&, ObjectPool*)) {
for (int i = 0; i < filter->values_size(); ++i) {
PColumnValue column = filter->values(i);
- assign_func(_context.hybrid_set, column, _pool);
+ assign_func(_context->hybrid_set, column, _pool);
}
}
- size_t get_in_filter_size() const { return _context.hybrid_set->size(); }
+ size_t get_in_filter_size() const {
+ return _context->hybrid_set ? _context->hybrid_set->size() : 0;
+ }
std::shared_ptr<BitmapFilterFuncBase> get_bitmap_filter() const {
- return _context.bitmap_filter_func;
+ return _context->bitmap_filter_func;
}
friend class IRuntimeFilter;
void set_filter_id(int id) {
- if (_context.bloom_filter_func) {
- _context.bloom_filter_func->set_filter_id(id);
+ if (_context->bloom_filter_func) {
Review Comment:
warning: method 'set_filter_id' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void set_filter_id(int id) {
```
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -794,27 +784,27 @@
int128_t max_val = StringParser::string_to_int<int128_t>(
max_string_val.c_str(), max_string_val.length(), &result);
DCHECK(result == StringParser::PARSE_SUCCESS);
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
}
case TYPE_FLOAT: {
float min_val =
static_cast<float>(minmax_filter->min_val().doubleval());
float max_val =
static_cast<float>(minmax_filter->max_val().doubleval());
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the
type name [modernize-use-auto]
```suggestion
auto max_val =
static_cast<float>(minmax_filter->max_val().doubleval());
```
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -794,27 +784,27 @@
int128_t max_val = StringParser::string_to_int<int128_t>(
max_string_val.c_str(), max_string_val.length(), &result);
DCHECK(result == StringParser::PARSE_SUCCESS);
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
}
case TYPE_FLOAT: {
float min_val =
static_cast<float>(minmax_filter->min_val().doubleval());
float max_val =
static_cast<float>(minmax_filter->max_val().doubleval());
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
}
case TYPE_DOUBLE: {
double min_val =
static_cast<double>(minmax_filter->min_val().doubleval());
double max_val =
static_cast<double>(minmax_filter->max_val().doubleval());
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the
type name [modernize-use-auto]
```suggestion
auto max_val =
static_cast<double>(minmax_filter->max_val().doubleval());
```
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -736,53 +727,52 @@
// assign this filter by protobuf
Status assign(const PBloomFilter* bloom_filter,
butil::IOBufAsZeroCopyInputStream* data,
bool contain_null) {
- _is_bloomfilter = true;
// we won't use this class to insert or find any data
// so any type is ok
-
_context.bloom_filter_func.reset(create_bloom_filter(_column_return_type ==
INVALID_TYPE
- ?
PrimitiveType::TYPE_INT
- :
_column_return_type));
- RETURN_IF_ERROR(_context.bloom_filter_func->assign(data,
bloom_filter->filter_length(),
- contain_null));
+
_context->bloom_filter_func.reset(create_bloom_filter(_column_return_type ==
INVALID_TYPE
+ ?
PrimitiveType::TYPE_INT
+ :
_column_return_type));
+ RETURN_IF_ERROR(_context->bloom_filter_func->assign(data,
bloom_filter->filter_length(),
+ contain_null));
return Status::OK();
}
// used by shuffle runtime filter
// assign this filter by protobuf
Status assign(const PMinMaxFilter* minmax_filter, bool contain_null) {
PrimitiveType type = to_primitive_type(minmax_filter->column_type());
- _context.minmax_func.reset(create_minmax_filter(type));
+ _context->minmax_func.reset(create_minmax_filter(type));
if (contain_null) {
- _context.minmax_func->set_null_aware(true);
- _context.minmax_func->set_contain_null();
+ _context->minmax_func->set_null_aware(true);
+ _context->minmax_func->set_contain_null();
}
switch (type) {
case TYPE_BOOLEAN: {
bool min_val = minmax_filter->min_val().boolval();
bool max_val = minmax_filter->max_val().boolval();
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
}
case TYPE_TINYINT: {
int8_t min_val =
static_cast<int8_t>(minmax_filter->min_val().intval());
int8_t max_val =
static_cast<int8_t>(minmax_filter->max_val().intval());
- return _context.minmax_func->assign(&min_val, &max_val);
+ return _context->minmax_func->assign(&min_val, &max_val);
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the
type name [modernize-use-auto]
```suggestion
auto max_val =
static_cast<int8_t>(minmax_filter->max_val().intval());
```
--
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
