-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix](window_func) fix bug of agg function used in window function and add many test cases (#40678) #41329
Conversation
…d add many test cases (apache#40678)
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
} | ||
|
||
template <WindowFunnelMode WINDOW_FUNNEL_MODE> | ||
int _match_event_list(size_t& start_row, size_t row_count, | ||
const NativeType* timestamp_data) const { | ||
int _match_event_list(size_t& start_row, size_t row_count) const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function '_match_event_list' has cognitive complexity of 64 (threshold 50) [readability-function-cognitive-complexity]
int _match_event_list(size_t& start_row, size_t row_count) const {
^
Additional context
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:167: +1, including nesting penalty of 0, nesting level increased to 1
if (match_row < row_count) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:177: +2, including nesting penalty of 1, nesting level increased to 2
for (; column_idx < event_count + 1 && match_row < row_count;
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:177: +1
for (; column_idx < event_count + 1 && match_row < row_count;
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:182: +3, including nesting penalty of 2, nesting level increased to 3
if constexpr (WINDOW_FUNNEL_MODE == WindowFunnelMode::FIXED) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:183: +4, including nesting penalty of 3, nesting level increased to 4
if (event_data[match_row] == 1) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:186: +5, including nesting penalty of 4, nesting level increased to 5
if (current_timestamp <= end_timestamp) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:194: +3, including nesting penalty of 2, nesting level increased to 3
if (match_row < row_count) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:198: +4, including nesting penalty of 3, nesting level increased to 4
if (is_matched) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:199: +5, including nesting penalty of 4, nesting level increased to 5
if constexpr (WINDOW_FUNNEL_MODE == WindowFunnelMode::INCREASE) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:203: +4, including nesting penalty of 3, nesting level increased to 4
if (!is_matched) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:206: +4, including nesting penalty of 3, nesting level increased to 4
if constexpr (WINDOW_FUNNEL_MODE == WindowFunnelMode::INCREASE) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:210: +4, including nesting penalty of 3, nesting level increased to 4
if constexpr (WINDOW_FUNNEL_MODE == WindowFunnelMode::DEDUPLICATION) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:212: +5, including nesting penalty of 4, nesting level increased to 5
if (match_row != last_match_row + 1) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:213: +6, including nesting penalty of 5, nesting level increased to 6
for (int tmp_column_idx = 1; tmp_column_idx < column_idx;
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:222: +7, including nesting penalty of 6, nesting level increased to 7
if (dup_match_row < match_row) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:228: +5, including nesting penalty of 4, nesting level increased to 5
if (is_dup) {
^
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:234: +1, nesting level increased to 3
} else {
^
} | ||
|
||
template <WindowFunnelMode WINDOW_FUNNEL_MODE> | ||
int _match_event_list(size_t& start_row, size_t row_count, | ||
const NativeType* timestamp_data) const { | ||
int _match_event_list(size_t& start_row, size_t row_count) const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: function '_match_event_list' exceeds recommended size/complexity thresholds [readability-function-size]
int _match_event_list(size_t& start_row, size_t row_count) const {
^
Additional context
be/src/vec/aggregate_functions/aggregate_function_window_funnel.h:153: 87 lines including whitespace and comments (threshold 80)
int _match_event_list(size_t& start_row, size_t row_count) const {
^
Proposed changes
Issue Number: close #xxx
BP #40678