summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Rowley <drowley@postgresql.org>2025-10-09 12:38:33 +1300
committerDavid Rowley <drowley@postgresql.org>2025-10-09 12:38:33 +1300
commita5a68dd6d5159626360f75ffde96eca879e6cc30 (patch)
treeb0e0d021cb79896e952b5fcf0c74e90db6b42d35
parent5e89985928795f243dc287210c2aa016dfd00bfe (diff)
Make truncate_useless_pathkeys() consider WindowFuncsHEADorigin/masterorigin/HEADmaster
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 <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAApHDvrj3HTKmXoLMbUjTO=_MNMxM=cnuCSyBKidAVibmYPnrg@mail.gmail.com
-rw-r--r--src/backend/optimizer/path/pathkeys.c24
-rw-r--r--src/test/regress/expected/window.out16
-rw-r--r--src/test/regress/sql/window.sql13
3 files changed, 51 insertions, 2 deletions
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 8b04d40d36d..879dcb4608e 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -2154,14 +2154,31 @@ right_merge_direction(PlannerInfo *root, PathKey *pathkey)
* Because we the have the possibility of incremental sort, a prefix list of
* keys is potentially useful for improving the performance of the requested
* ordering. Thus we return 0, if no valuable keys are found, or the number
- * of leading keys shared by the list and the requested ordering..
+ * of leading keys shared by the list and the requested ordering.
*/
static int
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 evaluation.
+ */
+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;
@@ -2278,6 +2295,9 @@ truncate_useless_pathkeys(PlannerInfo *root,
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);
if (nuseful2 > nuseful)
nuseful = nuseful2;
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index a2867f477f0..9e2f53726f5 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;