github-actions[bot] commented on code in PR #28094:
URL: https://github.com/apache/doris/pull/28094#discussion_r1418093760
##########
be/src/exprs/bloom_filter_func.h:
##########
@@ -217,70 +215,77 @@ class BloomFilterFuncBase : public FilterFuncBase {
bool _build_bf_exactly = false;
};
-struct BaseOp {
- virtual ~BaseOp() = default;
-
- virtual bool find_olap_engine(const BloomFilterAdaptor& bloom_filter,
- const void* data) const = 0;
-
- uint16_t find_batch_olap_engine_with_element_size(const
BloomFilterAdaptor& bloom_filter,
- const char* data, const
uint8* nullmap,
- uint16_t* offsets, int
number,
- const bool
is_parse_column,
- size_t element_size)
const {
- uint16_t new_size = 0;
- if (is_parse_column) {
- if (nullmap == nullptr) {
- for (int i = 0; i < number; i++) {
- uint16_t idx = offsets[i];
- if (!find_olap_engine(bloom_filter, data + element_size *
idx)) {
- continue;
- }
- offsets[new_size++] = idx;
+template <typename T, bool need_trim = false>
+uint16_t find_batch_olap_engine(const BloomFilterAdaptor& bloom_filter, const
char* data,
Review Comment:
warning: function 'find_batch_olap_engine' has cognitive complexity of 51
(threshold 50) [readability-function-cognitive-complexity]
```cpp
uint16_t find_batch_olap_engine(const BloomFilterAdaptor& bloom_filter,
const char* data,
^
```
<details>
<summary>Additional context</summary>
**be/src/exprs/bloom_filter_func.h:221:** nesting level increased to 1
```cpp
auto get_element = [](const char* input_data, int idx) {
^
```
**be/src/exprs/bloom_filter_func.h:222:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if constexpr (std::is_same_v<T, StringRef> && need_trim) {
^
```
**be/src/exprs/bloom_filter_func.h:227:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
while (size > 0 && data[size - 1] == '\0') {
^
```
**be/src/exprs/bloom_filter_func.h:227:** +1
```cpp
while (size > 0 && data[size - 1] == '\0') {
^
```
**be/src/exprs/bloom_filter_func.h:231:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/exprs/bloom_filter_func.h:237:** +1, including nesting penalty of
0, nesting level increased to 1
```cpp
if (is_parse_column) {
^
```
**be/src/exprs/bloom_filter_func.h:238:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if (nullmap == nullptr) {
^
```
**be/src/exprs/bloom_filter_func.h:239:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
for (int i = 0; i < number; i++) {
^
```
**be/src/exprs/bloom_filter_func.h:241:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
if (!bloom_filter.test_element(get_element(data, idx))) {
^
```
**be/src/exprs/bloom_filter_func.h:246:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/exprs/bloom_filter_func.h:247:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
for (int i = 0; i < number; i++) {
^
```
**be/src/exprs/bloom_filter_func.h:249:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
if (nullmap[idx]) {
^
```
**be/src/exprs/bloom_filter_func.h:252:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
if (!bloom_filter.test_element(get_element(data, idx))) {
^
```
**be/src/exprs/bloom_filter_func.h:258:** +1, nesting level increased to 1
```cpp
} else {
^
```
**be/src/exprs/bloom_filter_func.h:259:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if (nullmap == nullptr) {
^
```
**be/src/exprs/bloom_filter_func.h:260:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
for (int i = 0; i < number; i++) {
^
```
**be/src/exprs/bloom_filter_func.h:261:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
if (!bloom_filter.test_element(get_element(data, i))) {
^
```
**be/src/exprs/bloom_filter_func.h:266:** +1, nesting level increased to 2
```cpp
} else {
^
```
**be/src/exprs/bloom_filter_func.h:267:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
for (int i = 0; i < number; i++) {
^
```
**be/src/exprs/bloom_filter_func.h:268:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
if (nullmap[i]) {
^
```
**be/src/exprs/bloom_filter_func.h:271:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
if (!bloom_filter.test_element(get_element(data, i))) {
^
```
</details>
##########
be/src/exprs/bloom_filter_func.h:
##########
@@ -394,33 +388,16 @@
}
}
}
-
- static void insert(BloomFilterAdaptor& bloom_filter, const void* data) {
- const auto* value = reinterpret_cast<const StringRef*>(data);
- if (value) {
- bloom_filter.add_bytes(value->data, value->size);
- }
- }
-
- bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void*
data) const override {
- const auto* value = reinterpret_cast<const StringRef*>(data);
- return bloom_filter.test(*value);
- }
};
// We do not need to judge whether data is empty, because null will not appear
// when filer used by the storage engine
struct FixedStringFindOp : public StringFindOp {
- bool find_olap_engine(const BloomFilterAdaptor& bloom_filter,
- const void* input_data) const override {
- const auto* value = reinterpret_cast<const StringRef*>(input_data);
- int64_t size = value->size;
- const char* data = value->data;
- // CHAR type may pad the tail with \0, need to trim
- while (size > 0 && data[size - 1] == '\0') {
- size--;
- }
- return bloom_filter.test(StringRef(value->data, size));
+ uint16_t find_batch_olap_engine(const BloomFilterAdaptor& bloom_filter,
const char* data,
+ const uint8* nullmap, uint16_t* offsets,
int number,
+ const bool is_parse_column) {
+ return find_batch_olap_engine<StringRef, true>(bloom_filter, data,
nullmap, offsets, number,
+ is_parse_column);
Review Comment:
warning: method 'find_batch_olap_engine' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static uint16_t find_batch_olap_engine(const BloomFilterAdaptor&
bloom_filter, const char* data,
```
--
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
