-
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
[refactor](exec) refactor analytic operator to improve performance #46181
base: master
Are you sure you want to change the base?
Conversation
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
run buildall |
TPC-H: Total hot run time: 32773 ms
|
TeamCity be ut coverage result: |
run buildall |
TPC-H: Total hot run time: 32571 ms
|
TeamCity be ut coverage result: |
c2155d6
to
3cd5904
Compare
run buildall |
TPC-H: Total hot run time: 32757 ms
|
TeamCity be ut coverage result: |
run buildall |
2 similar comments
run buildall |
run buildall |
TPC-H: Total hot run time: 33981 ms
|
TPC-DS: Total hot run time: 194699 ms
|
TeamCity be ut coverage result: |
ClickBench: Total hot run time: 31.57 s
|
run buildall |
TPC-H: Total hot run time: 33810 ms
|
TeamCity be ut coverage result: |
run buildall |
TPC-H: Total hot run time: 33825 ms
|
TPC-DS: Total hot run time: 195996 ms
|
TeamCity be ut coverage result: |
ClickBench: Total hot run time: 31.36 s
|
run buildall |
TPC-H: Total hot run time: 33899 ms
|
TeamCity be ut coverage result: |
run buildall |
TPC-DS: Total hot run time: 185728 ms
|
ClickBench: Total hot run time: 30.46 s
|
TeamCity be ut coverage result: |
run p1 |
run buildall |
run buildall |
TPC-H: Total hot run time: 32606 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 192622 ms
|
ClickBench: Total hot run time: 30.93 s
|
run buildall |
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 32583 ms
|
TPC-DS: Total hot run time: 192536 ms
|
ClickBench: Total hot run time: 31.34 s
|
// For window frame `ROWS|RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING` | ||
_executor.get_next_impl = &AnalyticSinkLocalState::_get_next_for_partition; | ||
} else if (p._has_range_window) { | ||
if (!p._has_window_start && !p._has_window_end) { |
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.
why try to do the work in FE?better add the check in FE set !has_window? consider update
run buildall |
TPC-H: Total hot run time: 32978 ms
|
TPC-DS: Total hot run time: 185972 ms
|
ClickBench: Total hot run time: 31.91 s
|
TeamCity be ut coverage result: |
_destroy_agg_status(); | ||
_agg_arena_pool = nullptr; | ||
|
||
std::vector<vectorized::MutableColumnPtr> tmp_result_window_columns; |
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.
why not just clear?
|
||
private: | ||
friend class AnalyticSinkOperatorX; | ||
Status _execute_impl(); | ||
bool _get_next_for_partition(int64_t batch_rows, int64_t current_block_base_pos); |
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.
add comment for all get_next function !
} else { | ||
current_row_start = _current_row_position + _rows_start_offset; | ||
} | ||
// Make sure range_start <= range_end |
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.
what case happen? Fox exmaple in comment
_fn_place_ptr + _offsets_of_aggregate_states[i], | ||
&dst->get_nested_column()); | ||
} | ||
continue; |
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.
why not use if else ? what are you doing ?
return shared_state.all_block_end; | ||
// If the end is not greater than the start, the current window should be empty. | ||
// _current_window_empty = false; | ||
_current_window_empty = |
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.
if empty window? should we do add_range_single_place
agg exec? maybe unelss?
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)