Skip to content
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

Backport #58739 to 23.8: Fix stream partitioning in parallel window functions #58758

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/Interpreters/InterpreterSelectQuery.cpp
Expand Up @@ -2837,7 +2837,15 @@ void InterpreterSelectQuery::executeWindow(QueryPlan & query_plan)
// has suitable sorting. Also don't create sort steps when there are no
// columns to sort by, because the sort nodes are confused by this. It
// happens in case of `over ()`.
if (!window.full_sort_description.empty() && (i == 0 || !sortIsPrefix(window, *windows_sorted[i - 1])))
// Even if full_sort_description of both windows match, in case of different
// partitioning we need to add a SortingStep to reshuffle data in the streams.
bool need_sort = !window.full_sort_description.empty();
if (need_sort && i != 0)
{
need_sort = !sortIsPrefix(window, *windows_sorted[i - 1])
|| (settings.max_threads != 1 && window.partition_by.size() != windows_sorted[i - 1]->partition_by.size());
}
if (need_sort)
{
SortingStep::Settings sort_settings(*context);

Expand Down
12 changes: 10 additions & 2 deletions src/Planner/Planner.cpp
Expand Up @@ -803,9 +803,17 @@ void addWindowSteps(QueryPlan & query_plan,
* has suitable sorting. Also don't create sort steps when there are no
* columns to sort by, because the sort nodes are confused by this. It
* happens in case of `over ()`.
* Even if full_sort_description of both windows match, in case of different
* partitioning we need to add a SortingStep to reshuffle data in the streams.
*/
if (!window_description.full_sort_description.empty() &&
(i == 0 || !sortDescriptionIsPrefix(window_description.full_sort_description, window_descriptions[i - 1].full_sort_description)))

bool need_sort = !window_description.full_sort_description.empty();
if (need_sort && i != 0)
{
need_sort = !sortDescriptionIsPrefix(window_description.full_sort_description, window_descriptions[i - 1].full_sort_description)
|| (settings.max_threads != 1 && window_description.partition_by.size() != window_descriptions[i - 1].partition_by.size());
}
if (need_sort)
{
SortingStep::Settings sort_settings(*query_context);

Expand Down
@@ -0,0 +1,18 @@
sales 15000
sales 15000
sales 15000
sales 29400
sales 29400
sales 29400
sales 43800
sales 43800
sales 43800
sales 15000 5000
sales 15000 5000
sales 15000 5000
sales 29400 4800
sales 29400 4800
sales 29400 4800
sales 43800 4800
sales 43800 4800
sales 43800 4800
@@ -0,0 +1,32 @@
CREATE TABLE empsalary
(
`depname` LowCardinality(String),
`empno` UInt64,
`salary` Int32,
`enroll_date` Date
)
ENGINE = Memory;

insert into empsalary values ('sales',3,4800,'2007-08-01'), ('sales',1,5000,'2006-10-01'), ('sales',4,4800,'2007-08-08');


insert into empsalary values ('sales',3,4800,'2007-08-01'), ('sales',1,5000,'2006-10-01'), ('sales',4,4800,'2007-08-08');

insert into empsalary values ('sales',3,4800,'2007-08-01'), ('sales',1,5000,'2006-10-01'), ('sales',4,4800,'2007-08-08');

-- 1 window function

SELECT depname,
sum(salary) OVER (PARTITION BY depname order by empno) AS depsalary
FROM empsalary
order by depsalary;


-- 2 window functions with different window,
-- but result should be the same for depsalary

SELECT depname,
sum(salary) OVER (PARTITION BY depname order by empno) AS depsalary,
min(salary) OVER (PARTITION BY depname, empno order by enroll_date) AS depminsalary
FROM empsalary
order by depsalary;