github-actions[bot] commented on code in PR #24735:
URL: https://github.com/apache/doris/pull/24735#discussion_r1360059195
##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -1593,28 +1598,19 @@ void
SegmentIterator::_output_non_pred_columns(vectorized::Block* block) {
Status SegmentIterator::_read_columns_by_index(uint32_t nrows_read_limit,
uint32_t& nrows_read,
Review Comment:
warning: function '_read_columns_by_index' has cognitive complexity of 31
(threshold 25) [readability-function-cognitive-complexity]
```cpp
Status SegmentIterator::_read_columns_by_index(uint32_t nrows_read_limit,
uint32_t& nrows_read,
^
```
<details>
<summary>Additional context</summary>
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1601:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
do {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1606:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
if (!has_next_range) {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1613:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
if (set_block_rowid) {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1619:** +1, nesting
level increased to 2
```cpp
} else {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1628:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if (_read_range_info.ranges.empty()) {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1632:** +1, including
nesting penalty of 0, nesting level increased to 1
```cpp
if (_opts.runtime_state) {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1634:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
if (threshold == 0 || _read_range_info.ranges.size() <= threshold) {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1635:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
^
```
**be/src/common/status.h:538:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1635:** +4, including
nesting penalty of 3, nesting level increased to 4
```cpp
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
^
```
**be/src/common/status.h:540:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1636:** +1, nesting
level increased to 2
```cpp
} else {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1637:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_read_many_columns_data(_read_range_info.rowids));
^
```
**be/src/common/status.h:538:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1637:** +4, including
nesting penalty of 3, nesting level increased to 4
```cpp
RETURN_IF_ERROR(_read_many_columns_data(_read_range_info.rowids));
^
```
**be/src/common/status.h:540:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1639:** +1, nesting
level increased to 1
```cpp
} else {
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1640:** +2, including
nesting penalty of 1, nesting level increased to 2
```cpp
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
^
```
**be/src/common/status.h:538:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/olap/rowset/segment_v2/segment_iterator.cpp:1640:** +3, including
nesting penalty of 2, nesting level increased to 3
```cpp
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
^
```
**be/src/common/status.h:540:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
</details>
##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -1626,8 +1622,49 @@ Status SegmentIterator::_read_columns_by_index(uint32_t
nrows_read_limit, uint32
}
_split_row_ranges.emplace_back(std::pair {range_from, range_to});
- // if _opts.read_orderby_key_reverse is true, only read one range for
fast reverse purpose
+
+ _read_range_info.add(range_from, range_to);
} while (nrows_read < nrows_read_limit && !_opts.read_orderby_key_reverse);
+
+ if (_read_range_info.ranges.empty()) {
+ return Status::OK();
+ }
+
+ if (_opts.runtime_state) {
+ int32_t threshold =
_opts.runtime_state->query_options().inverted_index_read_mode_threshold;
+ if (threshold == 0 || _read_range_info.ranges.size() <= threshold) {
+
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
+ } else {
+ RETURN_IF_ERROR(_read_many_columns_data(_read_range_info.rowids));
+ }
+ } else {
+
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
+ }
+
+ _read_range_info.clear();
+
+ return Status::OK();
+}
+
+Status SegmentIterator::_read_continuous_columns_data(
+ const std::vector<std::pair<uint32_t, uint32_t>>& ranges) {
+ for (auto& range : ranges) {
+ RETURN_IF_ERROR(_seek_columns(_first_read_column_ids, range.first));
+ RETURN_IF_ERROR(_read_columns(_first_read_column_ids,
_current_return_columns,
+ range.second - range.first));
+ }
+ return Status::OK();
+}
+
+Status SegmentIterator::_read_many_columns_data(const std::vector<uint32_t>&
rowids) {
Review Comment:
warning: method '_read_many_columns_data' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status SegmentIterator::_read_many_columns_data(const
std::vector<uint32_t>& rowids) {
```
##########
be/src/olap/rowset/segment_v2/segment_iterator.h:
##########
@@ -99,6 +99,28 @@ struct ColumnPredicateInfo {
std::string query_op;
};
+struct ReadRangeInfo {
+ std::vector<uint32_t> rowids;
+ std::vector<std::pair<uint32_t, uint32_t>> ranges;
+
+ void init(int32_t capacity) {
+ rowids.reserve(capacity / 2);
+ ranges.reserve(capacity / 2);
+ }
+
+ inline void add(uint32_t range_from, uint32_t range_to) {
+ ranges.emplace_back(std::make_pair(range_from, range_to));
+ for (uint32_t i = range_from; i < range_to; i++) {
+ rowids.push_back(i);
+ }
+ }
+
+ inline void clear() {
Review Comment:
warning: method 'clear' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static inline void clear() {
```
##########
be/src/olap/rowset/segment_v2/segment_iterator.h:
##########
@@ -99,6 +99,28 @@ struct ColumnPredicateInfo {
std::string query_op;
};
+struct ReadRangeInfo {
+ std::vector<uint32_t> rowids;
+ std::vector<std::pair<uint32_t, uint32_t>> ranges;
+
+ void init(int32_t capacity) {
Review Comment:
warning: method 'init' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static void init(int32_t capacity) {
```
##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -1626,8 +1622,49 @@ Status SegmentIterator::_read_columns_by_index(uint32_t
nrows_read_limit, uint32
}
_split_row_ranges.emplace_back(std::pair {range_from, range_to});
- // if _opts.read_orderby_key_reverse is true, only read one range for
fast reverse purpose
+
+ _read_range_info.add(range_from, range_to);
} while (nrows_read < nrows_read_limit && !_opts.read_orderby_key_reverse);
+
+ if (_read_range_info.ranges.empty()) {
+ return Status::OK();
+ }
+
+ if (_opts.runtime_state) {
+ int32_t threshold =
_opts.runtime_state->query_options().inverted_index_read_mode_threshold;
+ if (threshold == 0 || _read_range_info.ranges.size() <= threshold) {
+
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
+ } else {
+ RETURN_IF_ERROR(_read_many_columns_data(_read_range_info.rowids));
+ }
+ } else {
+
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
+ }
+
+ _read_range_info.clear();
+
+ return Status::OK();
+}
+
+Status SegmentIterator::_read_continuous_columns_data(
Review Comment:
warning: method '_read_continuous_columns_data' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status SegmentIterator::_read_continuous_columns_data(
```
##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -1626,8 +1622,49 @@ Status SegmentIterator::_read_columns_by_index(uint32_t
nrows_read_limit, uint32
}
_split_row_ranges.emplace_back(std::pair {range_from, range_to});
- // if _opts.read_orderby_key_reverse is true, only read one range for
fast reverse purpose
+
+ _read_range_info.add(range_from, range_to);
} while (nrows_read < nrows_read_limit && !_opts.read_orderby_key_reverse);
+
+ if (_read_range_info.ranges.empty()) {
+ return Status::OK();
+ }
+
+ if (_opts.runtime_state) {
+ int32_t threshold =
_opts.runtime_state->query_options().inverted_index_read_mode_threshold;
+ if (threshold == 0 || _read_range_info.ranges.size() <= threshold) {
+
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
+ } else {
+ RETURN_IF_ERROR(_read_many_columns_data(_read_range_info.rowids));
+ }
+ } else {
+
RETURN_IF_ERROR(_read_continuous_columns_data(_read_range_info.ranges));
+ }
+
+ _read_range_info.clear();
+
+ return Status::OK();
+}
+
+Status SegmentIterator::_read_continuous_columns_data(
+ const std::vector<std::pair<uint32_t, uint32_t>>& ranges) {
Review Comment:
warning: all parameters should be named in a function
[readability-named-parameter]
```suggestion
const std::vector<std::pair<uint32_t, uint32_t> /*unused*/>& ranges)
{
```
##########
be/src/olap/rowset/segment_v2/segment_iterator.h:
##########
@@ -212,6 +234,9 @@ class SegmentIterator : public RowwiseIterator {
vectorized::MutableColumns&
column_block, size_t nrows);
[[nodiscard]] Status _read_columns_by_index(uint32_t nrows_read_limit,
uint32_t& nrows_read,
bool set_block_rowid);
+ [[nodiscard]] Status _read_continuous_columns_data(
+ const std::vector<std::pair<uint32_t, uint32_t>>& ranges);
Review Comment:
warning: parameter 1 is const-qualified in the function declaration;
const-qualification of parameters only has an effect in function definitions
[readability-avoid-const-params-in-decls]
```suggestion
std::vector<std::pair<uint32_t, uint32_t>>& ranges);
```
##########
be/src/olap/rowset/segment_v2/segment_iterator.h:
##########
@@ -99,6 +99,28 @@ struct ColumnPredicateInfo {
std::string query_op;
};
+struct ReadRangeInfo {
+ std::vector<uint32_t> rowids;
+ std::vector<std::pair<uint32_t, uint32_t>> ranges;
+
+ void init(int32_t capacity) {
+ rowids.reserve(capacity / 2);
+ ranges.reserve(capacity / 2);
+ }
+
+ inline void add(uint32_t range_from, uint32_t range_to) {
Review Comment:
warning: method 'add' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static inline void add(uint32_t range_from, uint32_t range_to) {
```
--
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]
