From dfcf6fa7749d1fc4660d03590f14dca7c855048c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 5 Oct 2026 12:52:48 -0400 Subject: [PATCH] Don't absorb child pathkeys into dummy AppendPaths for set-ops. create_append_path() has a rather questionable habit of overriding the caller-supplied pathkeys when it sees that there is a single child path, and applying the child's pathkeys instead. In most cases we can get away with that, but it does not work for set-operation AppendPaths. In set-operation nests, the append's result tlist will contain "varno 0" Vars, which won't match the child's pathkeys, leading to failure in create_plan(). We didn't have this problem before 03d40e4b5 allowed eliding provably-empty child nodes of set-ops; there would never have been a case with only one surviving child node, so create_append_path() would always have accepted the specified NIL pathkeys. As a band-aid fix, force the generated AppendPath's pathkeys to NIL even if create_append_path() did something else, thus restoring the status quo ante. This is demonstrably necessary for two of prepunion.c's three calls; I did it at the third too, although probably the pathkeys would already be NIL there. There have been reasons to want to get rid of the "varno 0" hack for a long time, and this is another one. But that will require significant surgery in prepunion.c, and likely some changes in parsetree representation, so we can't tackle it for v19. Hence, we need a band-aid. Bug: #19742 Reported-by: Junwen AN Author: Tom Lane Co-authored-by: shihao zhong Discussion: https://postgr.es/m/19742-dc403ca277cad1d3@postgresql.org Backpatch-through: 19 --- src/backend/optimizer/prep/prepunion.c | 16 ++++++ src/test/regress/expected/union.out | 80 +++++++++++++++++++++++++- src/test/regress/sql/union.sql | 29 +++++++++- 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index b136f12ff3b..02ac58be211 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -862,6 +862,17 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, apath = (Path *) create_append_path(root, result_rel, cheapest, NIL, NULL, 0, false, -1); + /* + * Although we told create_append_path to assign NIL pathkeys to the + * AppendPath, it may have overridden that (if there's just one surviving + * child path, it will use that path's pathkeys). However, createplan.c + * will fail because the append relation's tlist contains "varno 0" Vars + * (cf. generate_append_tlist), which won't match what is in the pathkeys. + * We need to fix that someday, but for now, just force the AppendPath's + * pathkeys back to NIL. + */ + apath->pathkeys = NIL; + /* * Initialize the result row estimate to the total input size. This is * correct for UNION ALL; for the UNION case it is overwritten below with @@ -908,6 +919,9 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, create_append_path(root, result_rel, partial, NIL, NULL, parallel_workers, enable_parallel_append, -1); + /* See comment above about pathkeys vs. "varno 0" Vars */ + papath->pathkeys = NIL; + gpath = (Path *) create_gather_path(root, result_rel, papath, result_rel->reltarget, NULL, NULL); @@ -1224,6 +1238,8 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root, apath = (Path *) create_append_path(root, result_rel, append, NIL, NULL, 0, false, -1); + /* See comment in generate_union_paths about "varno 0" Vars */ + apath->pathkeys = NIL; add_path(result_rel, apath); diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out index f07b6141e75..2495351d907 100644 --- a/src/test/regress/expected/union.out +++ b/src/test/regress/expected/union.out @@ -1366,7 +1366,7 @@ ORDER BY 1; Output: tenk1.two (5 rows) --- Try a mixed setop case. Ensure the right-hand UNION child gets removed. +-- Try mixed setop cases. Ensure the right-hand UNION child gets removed. EXPLAIN (COSTS OFF, VERBOSE) SELECT two FROM tenk1 t1 EXCEPT @@ -1388,6 +1388,84 @@ SELECT ten FROM tenk1 dummy WHERE 1=2; Output: t2.four (11 rows) +EXPLAIN (COSTS OFF, VERBOSE) +SELECT two FROM tenk1 t1 WHERE two = 1 +INTERSECT ALL +SELECT four FROM tenk1 t2 WHERE four = 1 +UNION +SELECT ten FROM tenk1 dummy WHERE 1=2; + QUERY PLAN +----------------------------------------- + HashAggregate + Output: t1.two + Group Key: t1.two + -> HashSetOp Intersect All + Output: t1.two + -> Seq Scan on public.tenk1 t1 + Output: t1.two + Filter: (t1.two = 1) + -> Seq Scan on public.tenk1 t2 + Output: t2.four + Filter: (t2.four = 1) +(11 rows) + +-- As above, but with sorting: ensure we do not get confused by child pathkeys +SET enable_hashagg = off; +EXPLAIN (COSTS OFF, VERBOSE) +SELECT two FROM tenk1 t1 WHERE two = 1 +INTERSECT ALL +SELECT four FROM tenk1 t2 WHERE four = 1 +UNION +SELECT ten FROM tenk1 dummy WHERE 1=2; + QUERY PLAN +----------------------------------------------------- + Unique + Output: t1.two + -> Sort + Output: t1.two + Sort Key: t1.two + -> SetOp Intersect All + Output: t1.two + -> Sort + Output: t1.two + Sort Key: t1.two + -> Seq Scan on public.tenk1 t1 + Output: t1.two + Filter: (t1.two = 1) + -> Sort + Output: t2.four + Sort Key: t2.four + -> Seq Scan on public.tenk1 t2 + Output: t2.four + Filter: (t2.four = 1) +(19 rows) + +-- also check case where the upper op is EXCEPT +EXPLAIN (COSTS OFF, VERBOSE) +(SELECT two FROM tenk1 t1 WHERE two = 1 + INTERSECT + SELECT four FROM tenk1 t2 WHERE four = 1) +EXCEPT ALL +SELECT ten FROM tenk1 dummy WHERE 1=2; + QUERY PLAN +----------------------------------------- + SetOp Intersect + Output: t1.two + -> Sort + Output: t1.two + Sort Key: t1.two + -> Seq Scan on public.tenk1 t1 + Output: t1.two + Filter: (t1.two = 1) + -> Sort + Output: t2.four + Sort Key: t2.four + -> Seq Scan on public.tenk1 t2 + Output: t2.four + Filter: (t2.four = 1) +(14 rows) + +RESET enable_hashagg; -- Ensure EXPLAIN can show a dummy set operation whose output is coerced -- to another type by the parent set operation. EXPLAIN (COSTS OFF, VERBOSE) diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql index a787dd25d23..8fce4528a3f 100644 --- a/src/test/regress/sql/union.sql +++ b/src/test/regress/sql/union.sql @@ -523,7 +523,7 @@ EXCEPT ALL SELECT four FROM tenk1 WHERE 1=2 ORDER BY 1; --- Try a mixed setop case. Ensure the right-hand UNION child gets removed. +-- Try mixed setop cases. Ensure the right-hand UNION child gets removed. EXPLAIN (COSTS OFF, VERBOSE) SELECT two FROM tenk1 t1 EXCEPT @@ -531,6 +531,33 @@ SELECT four FROM tenk1 t2 UNION SELECT ten FROM tenk1 dummy WHERE 1=2; +EXPLAIN (COSTS OFF, VERBOSE) +SELECT two FROM tenk1 t1 WHERE two = 1 +INTERSECT ALL +SELECT four FROM tenk1 t2 WHERE four = 1 +UNION +SELECT ten FROM tenk1 dummy WHERE 1=2; + +-- As above, but with sorting: ensure we do not get confused by child pathkeys +SET enable_hashagg = off; + +EXPLAIN (COSTS OFF, VERBOSE) +SELECT two FROM tenk1 t1 WHERE two = 1 +INTERSECT ALL +SELECT four FROM tenk1 t2 WHERE four = 1 +UNION +SELECT ten FROM tenk1 dummy WHERE 1=2; + +-- also check case where the upper op is EXCEPT +EXPLAIN (COSTS OFF, VERBOSE) +(SELECT two FROM tenk1 t1 WHERE two = 1 + INTERSECT + SELECT four FROM tenk1 t2 WHERE four = 1) +EXCEPT ALL +SELECT ten FROM tenk1 dummy WHERE 1=2; + +RESET enable_hashagg; + -- Ensure EXPLAIN can show a dummy set operation whose output is coerced -- to another type by the parent set operation. EXPLAIN (COSTS OFF, VERBOSE)