From d15b5b573923d0895245efa9a4376effc1c818f6 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 6 Oct 2025 16:17:53 +1300 Subject: [PATCH v1] Make truncate_useless_pathkeys() consider WindowFuncs truncate_useless_pathkeys() seems to have neglected to account for PathKeys that might be useful for WindowClause evaluation. Modify it so that it properly accounts for that. To make this work, we also modify pathkeys_useful_for_ordering() so it checks sort_pathkeys rather than query_pathkeys. This was a problem because nothing was checking sort_pathkeys, so if query_pathkeys got set to another value for some other operation, then nothing would account for PathKeys useful for the ORDER BY clause. --- src/backend/optimizer/path/pathkeys.c | 22 +++++++++++++++++++++- src/test/regress/expected/window.out | 16 ++++++++++++++++ src/test/regress/sql/window.sql | 13 +++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index 8b04d40d36d..f32cfb068a5 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -2161,7 +2161,24 @@ pathkeys_useful_for_ordering(PlannerInfo *root, List *pathkeys) { int n_common_pathkeys; - (void) pathkeys_count_contained_in(root->query_pathkeys, pathkeys, + (void) pathkeys_count_contained_in(root->sort_pathkeys, pathkeys, + &n_common_pathkeys); + + return n_common_pathkeys; +} + +/* + * pathkeys_useful_for_windowing + * Count the number of pathkeys that are useful for meeting the + * query's desired sort order for window function evalulation. + */ +static int +pathkeys_useful_for_windowing(PlannerInfo *root, List *pathkeys) +{ + int n_common_pathkeys; + + (void) pathkeys_count_contained_in(root->window_pathkeys, + pathkeys, &n_common_pathkeys); return n_common_pathkeys; @@ -2276,6 +2293,9 @@ truncate_useless_pathkeys(PlannerInfo *root, nuseful = pathkeys_useful_for_merging(root, rel, pathkeys); nuseful2 = pathkeys_useful_for_ordering(root, pathkeys); + if (nuseful2 > nuseful) + nuseful = nuseful2; + nuseful2 = pathkeys_useful_for_windowing(root, pathkeys); if (nuseful2 > nuseful) nuseful = nuseful2; nuseful2 = pathkeys_useful_for_grouping(root, pathkeys); diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out index a595fa28ce1..f04fb9b77e3 100644 --- a/src/test/regress/expected/window.out +++ b/src/test/regress/expected/window.out @@ -4537,6 +4537,22 @@ WHERE first_emp = 1 OR last_emp = 1; sales | 4 | 4800 | 08-08-2007 | 3 | 1 (6 rows) +CREATE INDEX empsalary_salary_empno_idx ON empsalary (salary, empno); +SET enable_seqscan = 0; +-- Ensure no sorting is done and that the IndexScan maintains all pathkeys +-- useful for the final sort order. +EXPLAIN (COSTS OFF) +SELECT salary, empno, row_number() OVER (ORDER BY salary) rn +FROM empsalary +ORDER BY salary, empno; + QUERY PLAN +--------------------------------------------------------------------- + WindowAgg + Window: w1 AS (ORDER BY salary ROWS UNBOUNDED PRECEDING) + -> Index Only Scan using empsalary_salary_empno_idx on empsalary +(3 rows) + +RESET enable_seqscan; -- cleanup DROP TABLE empsalary; -- test user-defined window function with named args and default args diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql index 85fc621c8db..37d837a2f66 100644 --- a/src/test/regress/sql/window.sql +++ b/src/test/regress/sql/window.sql @@ -1522,6 +1522,19 @@ SELECT * FROM FROM empsalary) emp WHERE first_emp = 1 OR last_emp = 1; +CREATE INDEX empsalary_salary_empno_idx ON empsalary (salary, empno); + +SET enable_seqscan = 0; + +-- Ensure no sorting is done and that the IndexScan maintains all pathkeys +-- useful for the final sort order. +EXPLAIN (COSTS OFF) +SELECT salary, empno, row_number() OVER (ORDER BY salary) rn +FROM empsalary +ORDER BY salary, empno; + +RESET enable_seqscan; + -- cleanup DROP TABLE empsalary; -- 2.43.0