-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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) #41328
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
2a2f3b1
to
c2b3cb7
Compare
run buildall |
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 {
^
TeamCity be ut coverage result: |
c2b3cb7
to
161b983
Compare
…d add many test cases (apache#40678)
161b983
to
c22e17e
Compare
run buildall |
TeamCity be ut coverage result: |
Proposed changes
Issue Number: close #xxx
BP #40678