From 68dad257291c8327bccbd7175c6ea62932bc1972 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 3 Nov 2026 12:59:14 +1300 Subject: [PATCH v2 1/2] Fix incorrect Assert in add_foreign_ordered_paths 12933dc60 (originally 66c0185a3) added setop_pathkeys, which can be set and applied to query_pathkeys for set operation child queries when the child query does not have other final pathkey requirements. add_foreign_ordered_paths() assumed that if the query had an ORDER BY that the query_pathkeys would be equal to sort_pathkeys. That's not true when query_pathkeys is set to setop_pathkeys. standard_qp_callback can do this when all PathKeys for the ORDER BY clause were marked as redundant. Here we relax the Assert to allow for sort_pathkeys to be empty. In this case it is technically ok to mark pushdown_safe as true so that postgresGetForeignUpperPaths() allows add_foreign_final_paths() to generate paths with the LIMIT clause pushed down. We don't adjust that here as that could cause plan changes in the backbranches. Let's just fix things enough so the Assert doesn't fail and do a proper fix in master. Backpatch to 17 where the setop_pathkeys field was added. Reported-by: Robins Tharakan Discussion: https://postgr.es/m/CAEP4nAwnSefmi4w+89XbHQ0f-0OixCYUJo9kVcbXSYL8L4rEPg@mail.gmail.com Backpatch-through: 17 --- .../postgres_fdw/expected/postgres_fdw.out | 26 +++++++++++++++++++ contrib/postgres_fdw/postgres_fdw.c | 16 ++++++------ contrib/postgres_fdw/sql/postgres_fdw.sql | 9 +++++++ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 115ebc7925f..fa5844aa9cd 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -1382,6 +1382,32 @@ SELECT * FROM ( Remote SQL: SELECT "C 1" FROM "S 1"."T 1" (8 rows) +-- Ensure the code works correctly when sort_pathkeys is empty due to +-- a redundant pathkey. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ( + (SELECT c1, c2 FROM ft1 WHERE c1 = 0 ORDER BY c1 LIMIT 1) + UNION + (SELECT c1, c2 FROM ft1 WHERE c1 = 0 ORDER BY c1) +) a; + QUERY PLAN +--------------------------------------------------------------------------------------------- + Unique + Output: ft1.c1, ft1.c2 + -> Sort + Output: ft1.c1, ft1.c2 + Sort Key: ft1.c1, ft1.c2 + -> Append + -> Limit + Output: ft1.c1, ft1.c2 + -> Foreign Scan on public.ft1 + Output: ft1.c1, ft1.c2 + Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" = 0)) + -> Foreign Scan on public.ft1 ft1_1 + Output: ft1_1.c1, ft1_1.c2 + Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" = 0)) +(14 rows) + -- =================================================================== -- JOIN queries -- =================================================================== diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 6757ee923cb..1acf506f6fa 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -8268,20 +8268,20 @@ add_foreign_ordered_paths(PlannerInfo *root, RelOptInfo *input_rel, merge_fdw_options(fpinfo, ifpinfo, NULL); /* - * If the input_rel is a base or join relation, we would already have - * considered pushing down the final sort to the remote server when - * creating pre-sorted foreign paths for that relation, because the - * query_pathkeys is set to the root->sort_pathkeys in that case (see - * standard_qp_callback()). + * If the input_rel is a base or join relation, + * add_paths_with_pathkeys_for_rel() already considered the creation of + * paths to suit query_pathkeys. sort_pathkeys can be NIL here despite + * the query having an ORDER BY clause, and query_pathkeys can be set to + * setop_pathkeys (see standard_qp_callback()). */ if (input_rel->reloptkind == RELOPT_BASEREL || input_rel->reloptkind == RELOPT_JOINREL) { - Assert(root->query_pathkeys == root->sort_pathkeys); + Assert(root->query_pathkeys == root->sort_pathkeys || + root->sort_pathkeys == NIL); - /* Safe to push down if the query_pathkeys is safe to push down */ + /* Safe to push down if the query_pathkeys are safe to push down */ fpinfo->pushdown_safe = ifpinfo->qp_is_pushdown_safe; - return; } diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 77c9a25a890..058caac503f 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -542,6 +542,15 @@ SELECT * FROM ( SELECT 2 AS type,c1 FROM ft2 ) a ORDER BY type; +-- Ensure the code works correctly when sort_pathkeys is empty due to +-- a redundant pathkey. +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ( + (SELECT c1, c2 FROM ft1 WHERE c1 = 0 ORDER BY c1 LIMIT 1) + UNION + (SELECT c1, c2 FROM ft1 WHERE c1 = 0 ORDER BY c1) +) a; + -- =================================================================== -- JOIN queries -- =================================================================== -- 2.53.0