Optimized processing of ORDER BY in window functions by excitoon · Pull Request #34632 · ClickHouse/ClickHouse

excitoon · 2022-02-16T07:51:33Z

Changelog category (leave one):

Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Optimized processing of ORDER BY in window functions.

Information about CI checks: https://clickhouse.tech/docs/en/development/continuous-integration/

Closes: #19289

src/Interpreters/ExpressionAnalyzer.cpp

src/Processors/QueryPlan/Optimizations/Optimizations.h

src/Interpreters/ExpressionAnalyzer.h

src/Processors/QueryPlan/Optimizations/reuseStorageOrdering.cpp

src/Processors/QueryPlan/WindowStep.h

src/Interpreters/InterpreterSelectQuery.cpp

alexey-milovidov · 2022-04-04T22:11:20Z

Please resolve conflicts.

novikd

Looks good. Please, add a test with over (partition by ... order by ...).

src/Interpreters/ExpressionAnalyzer.cpp

novikd · 2022-05-02T17:16:08Z

src/Interpreters/ExpressionAnalyzer.cpp

 }

-void makeWindowDescriptionFromAST(const Context & context,
+void ExpressionAnalyzer::makeWindowDescriptionFromAST(const Context & context_,


You can remove the first parameter because it's a member function now.

It is still static member and we can't easily make that because basically lots of code for that class is made through static functions in corresponding cpp file. And same in two other cases in InterpreterSelectQuery.cpp.

novikd · 2022-05-02T17:18:09Z

src/Interpreters/InterpreterSelectQuery.cpp

 }

-static SortDescription getSortDescription(const ASTSelectQuery & query, ContextPtr context)
+SortDescription InterpreterSelectQuery::getSortDescription(const ASTSelectQuery & query, ContextPtr context_)


Here you can remove the second parameter.

src/Interpreters/InterpreterSelectQuery.cpp

src/Interpreters/WindowDescription.h

src/Processors/QueryPlan/Optimizations/reuseStorageOrderingForWindowFunctions.cpp

filimonov · 2022-05-11T21:39:52Z

https://gist.github.com/filimonov/260c288b3233f675f260120c47f0f753

filimonov · 2022-05-16T14:58:34Z

High memory usage for UNBOUNDED PRECEDING + UNBOUNDED FOLLOWING looks expected: we can not start steaming out any row before the last row in the window will be read.

So we need to store all the seen rows + all the whole window.

But in practice, we sometimes see almost 3.5x memory usage.

select number, lagInFrame(number) OVER (ORDER BY number ASC ROWS
                 BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING) AS prev from test format Null;

-- Peak memory usage (for query): 11.81 GiB. -- while the data size is 3.2 Gb

That additional memory increase looks related to the block sizes (and their size prediction?)

set max_block_size = 256;
-- Peak memory usage (for query): 11.95 GiB.

SET max_block_size = 8192;
-- Peak memory usage (for query): 11.93 GiB.

set max_block_size = 65505; -- default
-- Peak memory usage (for query): 11.81 GiB.

SET max_block_size = 80000
-- Peak memory usage (for query): 9.58 GiB.

SET max_block_size = 90000
-- Peak memory usage (for query): 8.70 GiB.

SET max_block_size = 100000;
-- Peak memory usage (for query): 7.49 GiB.

SET max_block_size = 130000
-- Peak memory usage (for query): 6.50 GiB.

SET max_block_size = 524288
-- Peak memory usage (for query): 6.51 GiB.

novikd

LGTM, but needs to fix flaky check:

Stateless tests flaky check (address, actions) — Timeout, fail: 77, passed: 0

tests/queries/0_stateless/01655_plan_optimizations_optimize_read_in_window_order_long.sh

excitoon · 2022-05-24T07:36:03Z

+ for table in query_log zookeeper_log trace_log transactions_info_log
+ clickhouse-local --path /var/lib/clickhouse/ -q 'select * from system.query_log format TSVWithNamesAndTypes'
+ pigz
Code: 76. DB::Exception: Cannot lock file /var/lib/clickhouse/status. Another server instance in same directory is already running. (CANNOT_OPEN_FILE)

novikd · 2022-06-07T15:58:43Z

@Mergifyio update

mergify · 2022-06-07T15:59:41Z

update

❌ Pull request can't be updated with latest base branch changes

Details

Mergify needs the author permission to update the base branch of the pull request.
@excitoon-favorites needs to authorize modification on its head branch.
err-code: 9889F

excitoon · 2022-06-07T17:23:35Z

Rebased.

novikd · 2022-06-20T18:03:05Z

Integration tests (asan, actions) [2/3] — fail: 21, passed: 619, flaky: 3
Integration tests (release, actions) [2/2] — fail: 1, passed: 993, flaky: 0
Stress test (debug, actions) — Backward compatibility check: OOM killer (or signal 9)

Unrelated

Stress test (undefined, actions) — Killed by signal (in clickhouse-server.log)

Created an issue #38247

…processing

Window functions and GCP fixes ClickHouse#36944, ClickHouse#34632, ClickHouse#37659 and ClickHouse#37882

…processing

Window functions and GCP fixes ClickHouse#36944, ClickHouse#34632, ClickHouse#37659 and ClickHouse#37882

…processing

Window functions and GCP fixes ClickHouse#36944, ClickHouse#34632, ClickHouse#37659 and ClickHouse#37882

…processing

…4632, ClickHouse#37659 and ClickHouse#37882 Merge pull request ClickHouse#36944 from excitoon-favorites/better_exp_smooth Merge pull request ClickHouse#34632 from excitoon-favorites/optimizedprocessing Merge pull request ClickHouse#37659 from frew/master Support `batch_delete` capability for GCS Merge pull request ClickHouse#37882 from excitoon-favorites/nodeleteobjects Fixes for objects removal in `S3ObjectStorage`

robot-clickhouse added the pr-improvement Pull request with some product improvements label Feb 16, 2022

Enmk added the altinity label Feb 16, 2022

novikd self-assigned this Feb 21, 2022

excitoon force-pushed the optimizedprocessing branch from 6013df4 to 154e61e Compare March 2, 2022 14:44

excitoon marked this pull request as ready for review March 21, 2022 14:13

novikd requested changes Mar 21, 2022

View reviewed changes

alexey-milovidov marked this pull request as draft April 4, 2022 22:11

excitoon force-pushed the optimizedprocessing branch from 28be3af to 99fbfde Compare April 12, 2022 10:17

excitoon force-pushed the optimizedprocessing branch 4 times, most recently from 0c1da3d to 9effb26 Compare April 25, 2022 07:18

excitoon marked this pull request as ready for review April 27, 2022 09:50

excitoon requested a review from novikd April 27, 2022 09:55

novikd reviewed May 2, 2022

View reviewed changes

excitoon requested a review from novikd May 5, 2022 16:28

novikd approved these changes May 16, 2022

View reviewed changes

excitoon force-pushed the optimizedprocessing branch from 5a16111 to e218a9b Compare May 23, 2022 08:18

novikd reviewed May 23, 2022

View reviewed changes

tests/queries/0_stateless/01655_plan_optimizations_optimize_read_in_window_order_long.sh Outdated Show resolved Hide resolved

excitoon force-pushed the optimizedprocessing branch from e218a9b to 9f17929 Compare May 23, 2022 16:13

excitoon requested a review from novikd May 24, 2022 07:34

alexey-milovidov removed the altinity label May 25, 2022

excitoon force-pushed the optimizedprocessing branch from 9f17929 to ca69c57 Compare June 7, 2022 17:23

Fixed long test.

ea156fe

excitoon force-pushed the optimizedprocessing branch from ad5462b to ea156fe Compare June 20, 2022 02:15

novikd approved these changes Jun 20, 2022

View reviewed changes

novikd merged commit f6692c3 into ClickHouse:master Jun 20, 2022

excitoon pushed a commit to excitoon-favorites/ClickHouse that referenced this pull request Jun 24, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

317e11a

…processing

excitoon pushed a commit to Altinity/ClickHouse that referenced this pull request Jun 24, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

be0ecdc

…processing

excitoon pushed a commit to Altinity/ClickHouse that referenced this pull request Jun 27, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

8c38fb8

…processing

excitoon mentioned this pull request Jun 27, 2022

Window functions fixes ClickHouse/ClickHouse#36944 and ClickHouse/ClickHouse#34632 -- obsolete Altinity/ClickHouse#165

Closed

excitoon pushed a commit to Altinity/ClickHouse that referenced this pull request Jul 14, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

c860f72

…processing

arthurpassos added a commit to Altinity/ClickHouse that referenced this pull request Jul 15, 2022

Merge pull request #164 from Altinity/backport/22.3_36944_34632

97b66d6

Window functions and GCP fixes ClickHouse#36944, ClickHouse#34632, ClickHouse#37659 and ClickHouse#37882

Enmk pushed a commit to Altinity/ClickHouse that referenced this pull request Jul 28, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

1f52a06

…processing

Enmk pushed a commit to Altinity/ClickHouse that referenced this pull request Jul 28, 2022

Merge pull request #164 from Altinity/backport/22.3_36944_34632

38f7435

Window functions and GCP fixes ClickHouse#36944, ClickHouse#34632, ClickHouse#37659 and ClickHouse#37882

Enmk mentioned this pull request Jul 28, 2022

22.3.9 Pre-release PR Altinity/ClickHouse#181

Closed

quickhouse mentioned this pull request Jul 31, 2022

Fixed Unknown identifier (aggregate-function) exception which appears when a user tries to calculate WINDOW ORDER BY/PARTITION BY expressions over aggregate functions #39762

Merged

Enmk pushed a commit to Altinity/ClickHouse that referenced this pull request Aug 4, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

6f77382

…processing

Enmk pushed a commit to Altinity/ClickHouse that referenced this pull request Aug 4, 2022

Merge pull request #164 from Altinity/backport/22.3_36944_34632

797f3d3

Window functions and GCP fixes ClickHouse#36944, ClickHouse#34632, ClickHouse#37659 and ClickHouse#37882

This was referenced Aug 4, 2022

22.3.10 Pre-release PR Altinity/ClickHouse#186

Closed

22.3.10 Pre-release PR Altinity/ClickHouse#188

Merged

quickhouse mentioned this pull request Aug 4, 2022

Fixed Unknown identifier (aggregate-function) exception which appears when a user tries to calculate WINDOW ORDER BY/PARTITION BY expressions over aggregate functions — backport of ClickHouse/ClickHouse#39762 Altinity/ClickHouse#189

Merged

Enmk mentioned this pull request Sep 12, 2022

22.3.12 Pre-release Altinity/ClickHouse#199

Merged

4 tasks

Enmk pushed a commit to Altinity/ClickHouse that referenced this pull request Sep 13, 2022

Merge pull request ClickHouse#34632 from excitoon-favorites/optimized…

845b52d

…processing

Enmk mentioned this pull request Dec 22, 2022

22.3.15 Pre-release PR Altinity/ClickHouse#208

Merged

yl-lisen mentioned this pull request Mar 19, 2024

Fixes and improve more for aggregation timeplus-io/proton#614

Closed

25 tasks

Conversation

excitoon commented Feb 16, 2022 • edited by filimonov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexey-milovidov commented Apr 4, 2022

Uh oh!

novikd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

novikd May 2, 2022

Choose a reason for hiding this comment

Uh oh!

excitoon May 5, 2022

Choose a reason for hiding this comment

Uh oh!

novikd May 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

filimonov commented May 11, 2022

Uh oh!

filimonov commented May 16, 2022

Uh oh!

novikd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

excitoon commented May 24, 2022

Uh oh!

novikd commented Jun 7, 2022

Uh oh!

mergify bot commented Jun 7, 2022

❌ Pull request can't be updated with latest base branch changes

Uh oh!

excitoon commented Jun 7, 2022

Uh oh!

novikd commented Jun 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

excitoon commented Feb 16, 2022 •

edited by filimonov

Loading