Skip to content

Commit a5a68dd

Browse files
committed
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. Making this work required adjusting two things: 1. Change from checking query_pathkeys to check sort_pathkeys instead. 2. Add explicit check for window_pathkeys For #1, query_pathkeys gets set in standard_qp_callback() according to the sort order requirements for the first operation to be applied after the join planner is finished, so this changes depending on which upper planner operations a particular query needs. If the query has window functions and no GROUP BY, then query_pathkeys gets set to window_pathkeys. Before this change, this meant PathKeys useful for the ORDER BY were not accounted for in queries with window functions. Because of #1, #2 is now required so that we explicitly check to ensure we don't truncate away PathKeys useful for window functions. Author: David Rowley <[email protected]> Discussion: https://postgr.es/m/CAApHDvrj3HTKmXoLMbUjTO=_MNMxM=cnuCSyBKidAVibmYPnrg@mail.gmail.com
1 parent 5e89985 commit a5a68dd

File tree

3 files changed

+51
-2
lines changed

3 files changed

+51
-2
lines changed

src/backend/optimizer/path/pathkeys.c

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2154,14 +2154,31 @@ right_merge_direction(PlannerInfo *root, PathKey *pathkey)
21542154
* Because we the have the possibility of incremental sort, a prefix list of
21552155
* keys is potentially useful for improving the performance of the requested
21562156
* ordering. Thus we return 0, if no valuable keys are found, or the number
2157-
* of leading keys shared by the list and the requested ordering..
2157+
* of leading keys shared by the list and the requested ordering.
21582158
*/
21592159
static int
21602160
pathkeys_useful_for_ordering(PlannerInfo *root, List *pathkeys)
21612161
{
21622162
int n_common_pathkeys;
21632163

2164-
(void) pathkeys_count_contained_in(root->query_pathkeys, pathkeys,
2164+
(void) pathkeys_count_contained_in(root->sort_pathkeys, pathkeys,
2165+
&n_common_pathkeys);
2166+
2167+
return n_common_pathkeys;
2168+
}
2169+
2170+
/*
2171+
* pathkeys_useful_for_windowing
2172+
* Count the number of pathkeys that are useful for meeting the
2173+
* query's desired sort order for window function evaluation.
2174+
*/
2175+
static int
2176+
pathkeys_useful_for_windowing(PlannerInfo *root, List *pathkeys)
2177+
{
2178+
int n_common_pathkeys;
2179+
2180+
(void) pathkeys_count_contained_in(root->window_pathkeys,
2181+
pathkeys,
21652182
&n_common_pathkeys);
21662183

21672184
return n_common_pathkeys;
@@ -2276,6 +2293,9 @@ truncate_useless_pathkeys(PlannerInfo *root,
22762293

22772294
nuseful = pathkeys_useful_for_merging(root, rel, pathkeys);
22782295
nuseful2 = pathkeys_useful_for_ordering(root, pathkeys);
2296+
if (nuseful2 > nuseful)
2297+
nuseful = nuseful2;
2298+
nuseful2 = pathkeys_useful_for_windowing(root, pathkeys);
22792299
if (nuseful2 > nuseful)
22802300
nuseful = nuseful2;
22812301
nuseful2 = pathkeys_useful_for_grouping(root, pathkeys);

src/test/regress/expected/window.out

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4537,6 +4537,22 @@ WHERE first_emp = 1 OR last_emp = 1;
45374537
sales | 4 | 4800 | 08-08-2007 | 3 | 1
45384538
(6 rows)
45394539

4540+
CREATE INDEX empsalary_salary_empno_idx ON empsalary (salary, empno);
4541+
SET enable_seqscan = 0;
4542+
-- Ensure no sorting is done and that the IndexScan maintains all pathkeys
4543+
-- useful for the final sort order.
4544+
EXPLAIN (COSTS OFF)
4545+
SELECT salary, empno, row_number() OVER (ORDER BY salary) rn
4546+
FROM empsalary
4547+
ORDER BY salary, empno;
4548+
QUERY PLAN
4549+
---------------------------------------------------------------------
4550+
WindowAgg
4551+
Window: w1 AS (ORDER BY salary ROWS UNBOUNDED PRECEDING)
4552+
-> Index Only Scan using empsalary_salary_empno_idx on empsalary
4553+
(3 rows)
4554+
4555+
RESET enable_seqscan;
45404556
-- cleanup
45414557
DROP TABLE empsalary;
45424558
-- test user-defined window function with named args and default args

src/test/regress/sql/window.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,19 @@ SELECT * FROM
15221522
FROM empsalary) emp
15231523
WHERE first_emp = 1 OR last_emp = 1;
15241524

1525+
CREATE INDEX empsalary_salary_empno_idx ON empsalary (salary, empno);
1526+
1527+
SET enable_seqscan = 0;
1528+
1529+
-- Ensure no sorting is done and that the IndexScan maintains all pathkeys
1530+
-- useful for the final sort order.
1531+
EXPLAIN (COSTS OFF)
1532+
SELECT salary, empno, row_number() OVER (ORDER BY salary) rn
1533+
FROM empsalary
1534+
ORDER BY salary, empno;
1535+
1536+
RESET enable_seqscan;
1537+
15251538
-- cleanup
15261539
DROP TABLE empsalary;
15271540

0 commit comments

Comments
 (0)