agora inbox for pgsql-bugs@postgresql.org  
help / color / mirror / Atom feed
BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
14+ messages / 4 participants
[nested] [flat]

* BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-03 16:17  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 14+ messages in thread

From: PG Bug reporting form @ 2026-10-03 16:17 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: feasiblechart@gmail.com

The following bug has been logged on the website:

Bug reference:      19742
Logged by:          Junwen AN
Email address:      feasiblechart@gmail.com
PostgreSQL version: 19beta4
Operating system:   Linux
Description:        

Please see the repro. Seems like a regression; 19beta4 and the current main
branch both have this error raised, but 18.6 works fine. I ran it with psql

CREATE TABLE d (a int);
INSERT INTO d VALUES (1), (1), (2), (NULL), (3);
SELECT * FROM (SELECT a FROM d INTERSECT ALL SELECT a FROM d
               UNION ALL SELECT a FROM d WHERE false) s
WHERE a = 1;
--   ERROR:  XX000: could not find pathkey item to sort
(prepare_sort_from_pathkeys, createplan.c)

-- 18.6:            a = 1, 1
-- 19beta4 / main:  ERROR:  could not find pathkey item to sort
-- EXPLAIN (without ANALYZE) fails the same way: the error is raised while
planning.

Did some more digging with LLM, and it seems this works fine
-- ============ workaround: the same query without a sorted SetOp
============
SET enable_sort = off;            -- or enable_hashagg = on with statistics
that favour hashing
SELECT * FROM (SELECT a FROM d INTERSECT ALL SELECT a FROM d
               UNION ALL SELECT a FROM d WHERE false) s
WHERE a = 1;                      -- 1, 1 (HashSetOp Intersect All)
RESET enable_sort;







^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-03 21:53  Tom Lane <tgl@sss.pgh.pa.us>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 2 replies; 14+ messages in thread

From: Tom Lane @ 2026-10-03 21:53 UTC (permalink / raw)
  To: feasiblechart@gmail.com; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org

PG Bug reporting form <noreply@postgresql.org> writes:
> Please see the repro. Seems like a regression; 19beta4 and the current main
> branch both have this error raised, but 18.6 works fine. I ran it with psql

> CREATE TABLE d (a int);
> INSERT INTO d VALUES (1), (1), (2), (NULL), (3);
> SELECT * FROM (SELECT a FROM d INTERSECT ALL SELECT a FROM d
>                UNION ALL SELECT a FROM d WHERE false) s
> WHERE a = 1;
> --   ERROR:  XX000: could not find pathkey item to sort

Bisecting shows this started with

fdda78e361f136ec2b8de579b366c1e66bba1199 is the first bad commit
commit fdda78e361f136ec2b8de579b366c1e66bba1199
Author: David Rowley <drowley@postgresql.org>
Date:   Wed Nov 5 11:48:09 2025 +1300

    Fix possible usage of incorrect UPPERREL_SETOP RelOptInfo
    
    03d40e4b5 allowed dummy UNION [ALL] children to be removed from the plan
    by checking for is_dummy_rel().  That commit neglected to still account
    for the relids from the dummy rel so that the correct UPPERREL_SETOP
    RelOptInfo could be found and used for adding the Paths to.

I suspect that that commit just allowed reaching some pre-existing
mistake, but I've not dug into it.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 01:00  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 14+ messages in thread

From: Tom Lane @ 2026-10-04 01:00 UTC (permalink / raw)
  To: feasiblechart@gmail.com; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org

I wrote:
> Bisecting shows this started with
> fdda78e361f136ec2b8de579b366c1e66bba1199 is the first bad commit
> I suspect that that commit just allowed reaching some pre-existing
> mistake, but I've not dug into it.

After looking a bit closer, v18 produces this plan:

 Append  (cost=84.23..84.46 rows=14 width=4)
   ->  SetOp Intersect All  (cost=84.23..84.39 rows=13 width=4)
         ->  Sort  (cost=42.12..42.15 rows=13 width=4)
               Sort Key: d.a
               ->  Seq Scan on d  (cost=0.00..41.88 rows=13 width=4)
                     Filter: (a = 1)
         ->  Sort  (cost=42.12..42.15 rows=13 width=4)
               Sort Key: d_1.a
               ->  Seq Scan on d d_1  (cost=0.00..41.88 rows=13 width=4)
                     Filter: (a = 1)
   ->  Result  (cost=0.00..0.00 rows=0 width=0)
         One-Time Filter: false

v19/HEAD produce a Path that is equivalent to v18's except for two
things:

* The empty-query Result isn't there; evidently we figured out that
it's useless and tossed it.  So now the AppendPath has only one child.

* The AppendPath is marked as having pathkeys:

   :path.pathkeys (
      {PATHKEY 
      :pk_eclass 
         {EQUIVALENCECLASS 
         :ec_opfamilies (o 1976)
         :ec_collation 0 
         :ec_childmembers_size 0 
         :ec_members (
            {EQUIVALENCEMEMBER 
            :em_expr 
               {VAR 
               :varno 1 
               :varattno 1 
               :vartype 23 
               :vartypmod -1 
               :varcollid 0 
               :varnullingrels (b)
               :varlevelsup 0 
               :varreturningtype 0 
               :varnosyn 1 
               :varattnosyn 1 
               :location -1
               }
            :em_relids (b 1)
            ...

whereas in v18 it has nil pathkeys.  The immediate problem is that
create_append_plan calls prepare_sort_from_pathkeys to try to
create a representation of the pathkey in terms of the Append's
tlist, and what's in the Append's tlist is

         {VAR 
         :varno 0 
         :varattno 1 
         :vartype 23 
         :vartypmod -1 
         :varcollid 0 
         :varnullingrels (b)
         :varlevelsup 0 
         :varreturningtype 0 
         :varnosyn 0 
         :varattnosyn 1 
         :location -1
         }

that is the tlist has been translated to the "varno zero"
representation that prepunion.c generates.  So we fail to
match the 1/1 Var to this 0/1 Var, and kaboom.

So the seeds of this problem go far back, but the immediate
cause is that we're labeling the AppendPath with pathkeys
in cases where we did not before, and our implementation can't
actually support that.  Interestingly, this doesn't fail:

explain SELECT * FROM ((SELECT a FROM d INTERSECT ALL SELECT a FROM d)
union all (SELECT a FROM d INTERSECT ALL SELECT a FROM d)) s
WHERE a = 1;
                               QUERY PLAN                                
-------------------------------------------------------------------------
 Append  (cost=84.23..168.92 rows=26 width=4)
   ->  SetOp Intersect All  (cost=84.23..84.39 rows=13 width=4)
         ->  Sort  (cost=42.12..42.15 rows=13 width=4)
               Sort Key: d.a
               ->  Seq Scan on d  (cost=0.00..41.88 rows=13 width=4)
                     Filter: (a = 1)
         ->  Sort  (cost=42.12..42.15 rows=13 width=4)
               Sort Key: d_1.a
               ->  Seq Scan on d d_1  (cost=0.00..41.88 rows=13 width=4)
                     Filter: (a = 1)
   ->  SetOp Intersect All  (cost=84.23..84.39 rows=13 width=4)
         ->  Sort  (cost=42.12..42.15 rows=13 width=4)
               Sort Key: d_2.a
               ->  Seq Scan on d d_2  (cost=0.00..41.88 rows=13 width=4)
                     Filter: (a = 1)
         ->  Sort  (cost=42.12..42.15 rows=13 width=4)
               Sort Key: d_3.a
               ->  Seq Scan on d d_3  (cost=0.00..41.88 rows=13 width=4)
                     Filter: (a = 1)

and the reason it doesn't fail is that the AppendPath has nil pathkeys
in this case.  So (I speculate that) we never attached pathkeys to a
UNION ALL AppendPath before, and the reason we're trying to now has
something to do with having reduced the child list to a singleton.

I'm too tired to dig any further tonight.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 05:23  shihao zhong <zhong950419@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 14+ messages in thread

From: shihao zhong @ 2026-10-04 05:23 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: feasiblechart@gmail.com, David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org

Hi Tom,

> I suspect that that commit just allowed reaching some pre-existing
> mistake, but I've not dug into it.

Yes, I think the mistake is older.

The "WHERE false" path is removed, so the UNION ALL has one child
left, the INTERSECT.  For an Append with one child,
create_append_path() copies the child's pathkeys.  Later
create_append_plan() looks for those sort columns in the Append's
own targetlist.  It can't find them, and we get the error.

It can't find them because the SetOp's pathkeys are wrong. A sorted
SetOp reuses the pathkeys of its left input.  Here "a = 1" makes the
subquery skip its own sort , so we add a Sort on top of the subquery.
That Sort's pathkeys are built from the subquery's columns, not from
the SetOp's output columns.

There is a second way to get wrong pathkeys, with no SetOp at all.
When the column types differ, recurse_set_operations() adds a
projection but keeps the old pathkeys

(select a from d union select a from d)
    union all select a::numeric from d where false;

So I did not fix the SetOp.  0001 makes these one-child Appends drop
the child's pathkeys, which covers both cases.  The planner adds a
Sort above if it needs the order.  0002 adds tests for the three
queries.

This is the small fix, meant for 19 and master.

My first try, v1, only fixed the SetOp's pathkeys.  It fixed the
reported query but changed many plans, so I think it is too much for
19.

I think a better fix would make the pathkeys right in both places,
so the Append can keep them.  I can work on that for master if you
like.

Thanks,
Shihao

Attachments:

  [application/octet-stream] v2-0002-Add-tests-for-setop-Appends-left-with-a-single-ch.patch (3.6K, ../../CAGRkXqTK91Bca0Z7+d7CSENAM08LsqhNT8t5KAFRCEY0NzTsUQ@mail.gmail.com/3-v2-0002-Add-tests-for-setop-Appends-left-with-a-single-ch.patch)
  download | inline diff:
From 34787017912a7692d82bbc287ef42452ab30b254 Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Sat, 3 Oct 2026 22:00:46 -0600
Subject: [PATCH v2 2/2] Add tests for setop Appends left with a single child

Reported-by: Junwen AN <feasiblechart@gmail.com>
---
 src/test/regress/expected/union.out | 61 +++++++++++++++++++++++++++++
 src/test/regress/sql/union.sql      | 24 ++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 84abcd6b14f..ad2573b247b 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -1388,6 +1388,67 @@ SELECT ten FROM tenk1 dummy WHERE 1=2;
                      Output: t2.four
 (11 rows)
 
+-- Ensure the Append left with a single child after removing an empty input
+-- doesn't use the child's pathkeys.
+SET enable_hashagg = off;
+EXPLAIN (COSTS OFF)
+SELECT two FROM tenk1 WHERE two = 1
+INTERSECT ALL
+SELECT four FROM tenk1 WHERE four = 1
+UNION ALL
+SELECT ten FROM tenk1 WHERE 1=2;
+              QUERY PLAN               
+---------------------------------------
+ SetOp Intersect All
+   ->  Sort
+         Sort Key: tenk1.two
+         ->  Seq Scan on tenk1
+               Filter: (two = 1)
+   ->  Sort
+         Sort Key: tenk1_1.four
+         ->  Seq Scan on tenk1 tenk1_1
+               Filter: (four = 1)
+(9 rows)
+
+EXPLAIN (COSTS OFF)
+(SELECT two FROM tenk1 WHERE two = 1
+ INTERSECT
+ SELECT four FROM tenk1 WHERE four = 1)
+EXCEPT ALL
+SELECT ten FROM tenk1 WHERE 1=2;
+              QUERY PLAN               
+---------------------------------------
+ SetOp Intersect
+   ->  Sort
+         Sort Key: tenk1.two
+         ->  Seq Scan on tenk1
+               Filter: (two = 1)
+   ->  Sort
+         Sort Key: tenk1_1.four
+         ->  Seq Scan on tenk1 tenk1_1
+               Filter: (four = 1)
+(9 rows)
+
+-- As above, but the child's output needs a type coercion
+EXPLAIN (COSTS OFF)
+(SELECT two FROM tenk1 UNION SELECT four FROM tenk1)
+UNION ALL
+SELECT ten::numeric FROM tenk1 WHERE 1=2;
+                    QUERY PLAN                     
+---------------------------------------------------
+ Result
+   ->  Unique
+         ->  Merge Append
+               Sort Key: tenk1.two
+               ->  Sort
+                     Sort Key: tenk1.two
+                     ->  Seq Scan on tenk1
+               ->  Sort
+                     Sort Key: tenk1_1.four
+                     ->  Seq Scan on tenk1 tenk1_1
+(10 rows)
+
+RESET enable_hashagg;
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index c8de276c2b5..8a0d32e5400 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -531,6 +531,30 @@ SELECT four FROM tenk1 t2
 UNION
 SELECT ten FROM tenk1 dummy WHERE 1=2;
 
+-- Ensure the Append left with a single child after removing an empty input
+-- doesn't use the child's pathkeys.
+SET enable_hashagg = off;
+EXPLAIN (COSTS OFF)
+SELECT two FROM tenk1 WHERE two = 1
+INTERSECT ALL
+SELECT four FROM tenk1 WHERE four = 1
+UNION ALL
+SELECT ten FROM tenk1 WHERE 1=2;
+
+EXPLAIN (COSTS OFF)
+(SELECT two FROM tenk1 WHERE two = 1
+ INTERSECT
+ SELECT four FROM tenk1 WHERE four = 1)
+EXCEPT ALL
+SELECT ten FROM tenk1 WHERE 1=2;
+
+-- As above, but the child's output needs a type coercion
+EXPLAIN (COSTS OFF)
+(SELECT two FROM tenk1 UNION SELECT four FROM tenk1)
+UNION ALL
+SELECT ten::numeric FROM tenk1 WHERE 1=2;
+RESET enable_hashagg;
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
-- 
2.37.1 (Apple Git-137.1)



  [application/octet-stream] v2-0001-Don-t-let-single-child-setop-Appends-inherit-path.patch (1.8K, ../../CAGRkXqTK91Bca0Z7+d7CSENAM08LsqhNT8t5KAFRCEY0NzTsUQ@mail.gmail.com/4-v2-0001-Don-t-let-single-child-setop-Appends-inherit-path.patch)
  download | inline diff:
From 9f689d942e6ffa5f213367dfda3e1162a61b57be Mon Sep 17 00:00:00 2001
From: Shihao <zhong950419@gmail.com>
Date: Sat, 3 Oct 2026 22:00:46 -0600
Subject: [PATCH v2 1/2] Don't let single-child setop Appends inherit pathkeys

When all but one input of a UNION is proven empty, or the right
input of an EXCEPT ALL is, we use an Append with a single child.
create_append_path() copies the child's pathkeys in that case, but
in a setop tree those need not match the Append's targetlist, and
planning could fail with "could not find pathkey item to sort".
Clear the pathkeys of such Appends.

Bug: #19742
Reported-by: Junwen AN <feasiblechart@gmail.com>
---
 src/backend/optimizer/prep/prepunion.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index b136f12ff3b..9d0acad49f0 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -862,6 +862,12 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
 	apath = (Path *) create_append_path(root, result_rel, cheapest,
 										NIL, NULL, 0, false, -1);
 
+	/*
+	 * A single-child Append inherits its child's pathkeys, but those might
+	 * not match this setop's targetlist.
+	 */
+	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
@@ -1225,6 +1231,9 @@ generate_nonunion_paths(SetOperationStmt *op, PlannerInfo *root,
 													append, NIL, NULL, 0,
 													false, -1);
 
+				/* as in generate_union_paths, don't trust child pathkeys */
+				apath->pathkeys = NIL;
+
 				add_path(result_rel, apath);
 
 				return result_rel;
-- 
2.37.1 (Apple Git-137.1)



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 05:37  David Rowley <dgrowleyml@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: David Rowley @ 2026-10-04 05:37 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

On Sun, 4 Oct 2026 at 14:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> and the reason it doesn't fail is that the AppendPath has nil pathkeys
> in this case.  So (I speculate that) we never attached pathkeys to a
> UNION ALL AppendPath before, and the reason we're trying to now has
> something to do with having reduced the child list to a singleton.

It looks like a bug in add_setop_child_rel_equivalences(). It wrongly
assumes that setop_pathkeys will contain a PathKey for each tlist
entry.  The problem query has a redundant PathKey due to the WHERE a =
1.

I think the fix needs to be either don't remove redundant pathkeys for
setop_pathkeys or use some other method to figure out which
expressions to add in add_child_eq_member().

David





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 05:42  Tom Lane <tgl@sss.pgh.pa.us>
  parent: David Rowley <dgrowleyml@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2026-10-04 05:42 UTC (permalink / raw)
  To: David Rowley <dgrowleyml@gmail.com>; +Cc: feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

David Rowley <dgrowleyml@gmail.com> writes:
> I think the fix needs to be either don't remove redundant pathkeys for
> setop_pathkeys or use some other method to figure out which
> expressions to add in add_child_eq_member().

My own thoughts were along the lines of "don't ever assign pathkeys to
an AppendPath"; not sure if that's equivalent to your first idea.

In the long run I'd like to get rid of the varno-zero business
in favor of some less-magic representation; but that's clearly
not reasonable for v19.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 17:01  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2026-10-04 17:01 UTC (permalink / raw)
  To: David Rowley <dgrowleyml@gmail.com>; +Cc: feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

I wrote:
> My own thoughts were along the lines of "don't ever assign pathkeys to
> an AppendPath"; not sure if that's equivalent to your first idea.

Concretely, the attached fixes the given test case.  There are other
calls to create_append_path in prepunion.c, and I think they may
all need to do likewise, but I didn't analyze them.

It'd be nominally cleaner to add a flag to create_append_path telling
it whether it's allowed to override the given pathkeys.  I didn't do
that here because it seems like this is a localized problem that
should eventually be fixed inside prepunion.c, but there's room to
argue differently.

			regards, tom lane

Attachments:

  [text/x-diff] wip-fix-bad-pathkeys-for-UNION-append.patch (1.0K, ../../401041.1791133299@sss.pgh.pa.us/2-wip-fix-bad-pathkeys-for-UNION-append.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index b136f12ff3b..c4e95f14dc5 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

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 18:41  shihao zhong <zhong950419@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: shihao zhong @ 2026-10-04 18:41 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: David Rowley <dgrowleyml@gmail.com>; feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

Hi Tom,

> There are other calls to create_append_path in prepunion.c, and I
> think they may all need to do likewise, but I didn't analyze them.

The EXCEPT ALL one needs it too.  With only your patch this still
fails:
    set enable_hashagg = off;
    (select a from d where a = 1 intersect select a from d where a = 1)
    except all select a from d where false;

The v2-0001 I posted upthread changes both places.  The partial
Append looks safe, its children always have NIL pathkeys.

Thanks,
Shihao

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 19:39  Tom Lane <tgl@sss.pgh.pa.us>
  parent: shihao zhong <zhong950419@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2026-10-04 19:39 UTC (permalink / raw)
  To: shihao zhong <zhong950419@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

shihao zhong <zhong950419@gmail.com> writes:
> Hi Tom,
>> There are other calls to create_append_path in prepunion.c, and I
>> think they may all need to do likewise, but I didn't analyze them.

> The EXCEPT ALL one needs it too.  With only your patch this still
> fails:
>     set enable_hashagg = off;
>     (select a from d where a = 1 intersect select a from d where a = 1)
>     except all select a from d where false;

Ah.  I'd been trying to make a test case for that one, but I didn't
realize that two levels of setop are required.  Something like this
doesn't fail:

explain
select * from
((select * from int8_tbl i81 order by q1)
except all (select * from int8_tbl i81b where false)) ss1,
((select * from int8_tbl i82 order by q1)
except all (select * from int8_tbl i82b where false)) ss2
where ss1.q1 = ss2.q1
;

However, digging into the guts of that doesn't leave a warm feeling
either.  Unpatched, we end up with the same situation where a
single-child AppendPath has a tlist containing varno-0 Vars and a
pathkey, and create_append_plan tries to compute sort column info from
that.  The reason it fails to fail is that *the equivalence classes
contain varno-0 Vars too*.  I've not entirely figured out why this
is different from the original test case --- well, okay, UNION ALL
at the top level is different because it doesn't make any pathkeys,
but if you change that to UNION the test case still fails on unpatched
code, and in that case the pathkey-slinging sure looks the same.

Regardless of the detailed reason for that, having varno-0 Vars in
equivalence classes scares the dickens out of me.  Each setop node
is going to have its own varno-0 Vars, and if they match on type then
the equivalence class machinery can't tell them apart, so it sure
seems like we are at risk of drawing false conclusions about whether
different setop outputs are sorted alike.  In the above example I was
trying to break it by having it falsely deduce that the outer WHERE
clause could be thrown away or reduced to an IS NOT NULL test.
I failed, which turns out to be because the questionable eclasses are
not at top level but within the two subroots associated with the two
setop nests.  So I think the potential bad effects are limited to
maybe mistakenly planning a single setop nest, and so far we've
escaped issues mainly because we don't do that much optimization of
non-UNION-ALL nests.  But it's really past time to get rid of the
varno-0 representation.  For now, one reason I like forcing these
paths' pathkeys to nil is that it limits the amount of damage that
could be done by false equivalence-class reasoning.  In particular,
I wonder whether v19/HEAD are at risk of such bugs in cases that
couldn't occur before we started eliding dummy child setops.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-04 21:43  shihao zhong <zhong950419@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: shihao zhong @ 2026-10-04 21:43 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: David Rowley <dgrowleyml@gmail.com>; feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

Hi Tom,

> In particular, I wonder whether v19/HEAD are at risk of such bugs in
> cases that couldn't occur before we started eliding dummy child
> setops.

I looked for one with random queries.  The attached script builds
nested setops with empty arms and mixed int and numeric columns,
puts an ORDER BY, a join or a second setop nest above them, and
checks the output against an answer worked out in Python. Obviously
I worked with Cladue to get that script generated.

I ran 20000 queries, each with seven sets of planner settings.  On
HEAD 2516 of the 140000 runs hit the pathkey error.  With both
Appends forced to NIL pathkeys none did.  There were no wrong
answers in either build.

It did find one more varno 0 problem, in EXPLAIN only.  18 is fine.
    create table d1 (a int);
    create table d2 (b numeric);
    explain (verbose, costs off)
    select b from d2 except
    (select a from d1 where false except all select a from d1);
    ERROR:  bogus varno: 0

928df067d1e handles a plain Var in the dummy Result's tlist.  Here
the Var is under an int to numeric cast, so set_plan_refs() misses
it.

The attached patch walks the whole expression.  It keeps the
varno 1 rewrite, so for a nested setop the name shown can come from
another child, as in the new test's output.

Thanks,
Shihao

Attachments:

  [text/x-python-script] setop_check.py (9.4K, ../../CAGRkXqTFwygKmjLG_Y=kbXRHsePFmD6k+qrzVLP4_KrG+-=oRg@mail.gmail.com/3-setop_check.py)
  download | inline:
#!/usr/bin/env python3
"""
Random test for the planning of set operations.

Makes random queries with nested UNION, INTERSECT and EXCEPT, runs each one
under several planner settings, and compares the output with an answer
worked out in Python from the table contents.  A run fails if EXPLAIN or
the query fails, if the rows are wrong, or if ORDER BY output is not sorted.

usage: setop_check.py N SEED        run N random queries
       setop_check.py N SEED ID     print query ID, expected rows and plans
Runs psql, so set PGHOST, PGPORT, PGDATABASE.  Creates tables d1 and d2.
"""
import random
import subprocess
import sys
from collections import Counter
from multiprocessing import Pool

SETUP = """
DROP TABLE IF EXISTS d1, d2;
CREATE TABLE d1 (a int, b int);
CREATE TABLE d2 (a int, b numeric);
INSERT INTO d1 SELECT nullif(g % 8, 7), nullif(g % 6, 5)
  FROM generate_series(1, 300) g;
INSERT INTO d2 SELECT nullif(g % 7, 6), nullif(g % 5, 4)
  FROM generate_series(1, 200) g;
CREATE INDEX ON d1 (a, b);
CREATE INDEX ON d2 (a);
ANALYZE d1, d2;
"""
NOHASH = "SET enable_hashagg = off; "
PARALLEL = ("SET parallel_setup_cost = 0; SET parallel_tuple_cost = 0; "
            "SET min_parallel_table_scan_size = 0; "
            "SET max_parallel_workers_per_gather = 2; ")
CONFIGS = {             # every query runs once under each of these
    "default": "",
    "nohash": NOHASH,
    "nohash_noseq": NOHASH + "SET enable_seqscan = off;",
    "nosort": "SET enable_sort = off; SET enable_incremental_sort = off;",
    "merge": NOHASH + "SET enable_hashjoin = off; SET enable_nestloop = off;",
    "parallel": PARALLEL,
    "parallel_nohash": PARALLEL + NOHASH,
}
NODES = ["SetOp", "Merge Append", "Parallel Append", "Merge Join", "WindowAgg"]
MARK = "@@"             # separates statement results in the psql output
TABLES = {}             # table name -> list of (a, b) rows

def psql(script):
    """Run a script, return its output lines."""
    p = subprocess.run(["psql", "-X", "-q", "-At", "-F", ","], input=script,
                       capture_output=True, text=True)
    return p.stdout.split("\n")[:-1]

def parse(lines):
    """psql output lines to rows.  A NULL is printed as an empty string."""
    return [tuple(int(f) if f else None for f in line.split(","))
            for line in lines]

# Parts of a leaf SELECT: the SQL text, and the same thing in Python.
COLUMN1 = [("a", lambda a, b: a), ("b", lambda a, b: b),
           ("a + 1", lambda a, b: None if a is None else a + 1),
           ("1", lambda a, b: 1)]
COLUMN2 = [("b", lambda a, b: b), ("a", lambda a, b: a),
           ("2", lambda a, b: 2), ("NULL::int", lambda a, b: None)]
FILTERS = [("true", lambda a, b: True), ("true", lambda a, b: True),
           ("a = 1", lambda a, b: a == 1),
           ("a < 3", lambda a, b: a is not None and a < 3),
           ("a IS NULL", lambda a, b: a is None),
           ("false", lambda a, b: False), ("1 = 2", lambda a, b: False)]

# A result is a Counter mapping each row to how many times it appears.
# Set operations treat NULLs as equal, like Python does with None.
SETOPS = {
    "UNION ALL": lambda l, r: l + r,
    "UNION": lambda l, r: Counter(set(l + r)),
    "INTERSECT ALL": lambda l, r: l & r,
    "INTERSECT": lambda l, r: Counter(set(l & r)),
    "EXCEPT ALL": lambda l, r: l - r,
    "EXCEPT": lambda l, r: Counter(set(l) - set(r)),
}

def leaf(rnd, ncols):
    """SELECT from one table.  Returns (sql, Counter of rows)."""
    table = rnd.choice(["d1", "d2"])
    cols = [rnd.choice(COLUMN1), rnd.choice(COLUMN2)][:ncols]
    where_sql, where = rnd.choice(FILTERS)
    sql = "SELECT %s FROM %s WHERE %s" % (
        ", ".join("%s AS x%d" % (c[0], i + 1) for i, c in enumerate(cols)),
        table, where_sql)
    if rnd.random() < 0.15:
        sql += " ORDER BY 1"                # does not change the rows
    return sql, Counter(tuple(c[1](a, b) for c in cols)
                        for a, b in TABLES[table] if where(a, b))

def tree(rnd, ncols, depth):
    """A set operation over two smaller trees, or a leaf."""
    if depth == 0:
        return leaf(rnd, ncols)
    op = rnd.choice(list(SETOPS))
    lsql, lrows = tree(rnd, ncols, depth - rnd.choice([1, 1, 1, depth]))
    rsql, rrows = tree(rnd, ncols, depth - rnd.choice([1, 1, 1, depth]))
    return "(%s) %s (%s)" % (lsql, op, rsql), SETOPS[op](lrows, rrows)

def sort_key(row):
    """Python sort key that matches ORDER BY, which puts NULLs last."""
    return [(v is None, v or 0) for v in row]

def make_query(rnd, qid):
    """Put something above a set operation that depends on its output."""
    ncols = rnd.choice([1, 2])
    sql, counter = tree(rnd, ncols, rnd.choice([1, 2, 3]))
    rows = list(counter.elements())
    first = Counter(row[0] for row in rows)     # first column -> count
    all_columns = ", ".join(str(i + 1) for i in range(ncols))
    order = None                                # "asc", "desc" or None
    shape = rnd.choice(["plain", "order", "order desc", "limit", "group",
                        "window", "in", "join table", "join setop"])
    if shape == "order":
        sql, order = sql + " ORDER BY " + all_columns, "asc"
    elif shape == "order desc":
        sql, order = sql + " ORDER BY %s DESC" % all_columns.replace(
            ", ", " DESC, "), "desc"
    elif shape == "limit":
        sql, order = sql + " ORDER BY %s LIMIT 7" % all_columns, "asc"
        rows = sorted(rows, key=sort_key)[:7]
    elif shape == "group":
        sql = "SELECT x1, count(*) FROM (%s) s GROUP BY x1 ORDER BY 1, 2" % sql
        rows, order = list(first.items()), "asc"
    elif shape == "window":
        sql = "SELECT x1, count(*) OVER (PARTITION BY x1) FROM (%s) s" % sql
        rows = [(row[0], first[row[0]]) for row in rows]
    elif shape == "in":
        sql = "SELECT a, b FROM d1 WHERE a IN (SELECT x1 FROM (%s) s)" % sql
        rows = [(a, b) for a, b in TABLES["d1"]
                if a is not None and a in first]
    elif shape == "join table":
        sql = "SELECT s.x1, d1.b FROM (%s) s JOIN d1 ON s.x1 = d1.a" % sql
        rows = [(row[0], b) for row in rows for a, b in TABLES["d1"]
                if a is not None and a == row[0]]
    elif shape == "join setop":
        sql2, counter2 = tree(rnd, 1, rnd.choice([1, 2]))
        sql = ("SELECT s1.x1 FROM (%s) s1, (%s) s2 WHERE s1.x1 = s2.x1"
               % (sql, sql2))
        rows = [(row[0],) for row in rows if row[0] is not None
                for _ in range(counter2[(row[0],)])]
    return {"id": qid, "sql": sql, "expect": rows, "order": order}

def check(test, got):
    if Counter(got) != Counter(test["expect"]):
        return "WRONG ROWS"
    keys = [sort_key(row) for row in got]
    if test["order"] == "desc":
        keys.reverse()
    if test["order"] and keys != sorted(keys):
        return "WRONG ORDER"
    return "ok"

def run(test):
    """Returns (query id, [(config, verdict, plan lines)])."""
    script = ""
    for settings in CONFIGS.values():
        script += "RESET ALL; %s\n" % settings
        for stmt in "EXPLAIN (VERBOSE, COSTS OFF) " + test["sql"], test["sql"]:
            # after each statement, print its SQLSTATE and error message
            script += "%s;\n\\echo %s:SQLSTATE :LAST_ERROR_MESSAGE\n" % (
                stmt, MARK)
    outputs, lines = [], []         # one (lines, status) per statement
    for line in psql(script):
        if line.startswith(MARK):
            outputs.append((lines, line[len(MARK):]))
            lines = []
        else:
            lines.append(line)
    outputs += [([], "connection lost")] * (2 * len(CONFIGS) - len(outputs))
    results = []
    for i, config in enumerate(CONFIGS):
        (plan, explain_status), (rows, status) = outputs[2 * i:2 * i + 2]
        if not status.startswith("00000"):
            verdict = "ERROR: " + status
        else:
            verdict = check(test, parse(rows))
        if verdict == "ok" and not explain_status.startswith("00000"):
            verdict = "EXPLAIN ERROR: " + explain_status
        results.append((config, verdict, plan))
    return test["id"], results

def main(n, seed, show=None):
    # Create the tables, and read them back so Python sees the same data.
    psql(SETUP)
    for table in "d1", "d2":
        TABLES[table] = parse(psql("SELECT a, b FROM %s" % table))
    rnd = random.Random(seed)
    tests = [make_query(rnd, qid) for qid in range(n)]
    if show is not None:
        test = tests[show]
        print(test["sql"])
        print("\nexpected rows:", sorted(test["expect"], key=sort_key))
        for config, verdict, plan in run(test)[1]:
            print("\n-- %s: %s\n%s" % (config, verdict, "\n".join(plan)))
        return
    stats, failed = Counter(), []
    with Pool(8) as pool:
        for qid, results in pool.imap_unordered(run, tests, chunksize=8):
            stats["queries"] += 1
            stats["queries, expected result not empty"] += bool(
                tests[qid]["expect"])
            if any(verdict != "ok" for _, verdict, _ in results):
                failed.append(qid)
            for _, verdict, plan in results:
                stats["runs: " + verdict] += 1
                stats.update("runs with %s in the plan" % node
                             for node in NODES if any(
                                 line.lstrip(" ->").startswith(node)
                                 for line in plan))
    for key in sorted(stats):
        print("%7d  %s" % (stats[key], key))
    print("failed query ids:", *sorted(failed)[:50])

if __name__ == "__main__":
    main(*map(int, sys.argv[1:]))

  [application/octet-stream] check-head-seed19742.out (981B, ../../CAGRkXqTFwygKmjLG_Y=kbXRHsePFmD6k+qrzVLP4_KrG+-=oRg@mail.gmail.com/4-check-head-seed19742.out)
  download

  [application/octet-stream] check-v2-seed19742.out (1.0K, ../../CAGRkXqTFwygKmjLG_Y=kbXRHsePFmD6k+qrzVLP4_KrG+-=oRg@mail.gmail.com/5-check-v2-seed19742.out)
  download

  [application/octet-stream] v1-0003-Fix-EXPLAIN.patch (5.3K, ../../CAGRkXqTFwygKmjLG_Y=kbXRHsePFmD6k+qrzVLP4_KrG+-=oRg@mail.gmail.com/6-v1-0003-Fix-EXPLAIN.patch)
  download | inline diff:
From dab5511d645953b5ff4335375a68ce16897a01d6 Mon Sep 17 00:00:00 2001
From: shihao zhong <zhong950419@gmail.com>
Date: Sun, 4 Oct 2026 17:29:15 -0400
Subject: [PATCH v1] Fix EXPLAIN of a dummy set operation under a coercion

928df067d1e changed varno 0 Vars in a dummy Result's targetlist to
varno 1, but only when the Var was the whole expression.  A parent
set operation can put a type coercion above the Var, and then EXPLAIN
failed with "bogus varno: 0".  Fix the Vars inside expressions too.

Discussion: https://postgr.es/m/19742-dc403ca277cad1d3@postgresql.org
---
 src/backend/optimizer/plan/setrefs.c | 53 ++++++++++++++++++++--------
 src/test/regress/expected/union.out  | 20 +++++++++++
 src/test/regress/sql/union.sql       |  9 +++++
 3 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 8a641402a96..c1a0f1d10a6 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -155,6 +155,7 @@ static Plan *set_mergeappend_references(PlannerInfo *root,
 										int rtoffset);
 static void set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset);
 static Relids offset_relid_set(Relids relids, int rtoffset);
+static Node *fix_dummy_setop_vars_mutator(Node *node, void *context);
 static Node *fix_scan_expr(PlannerInfo *root, Node *node,
 						   int rtoffset, double num_exec);
 static Node *fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context);
@@ -1060,27 +1061,23 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 					 * Here we rewrite these to use varno==1, which is the
 					 * varno of the first set-op child.  Without this, EXPLAIN
 					 * will have trouble displaying targetlists of dummy set
-					 * operations.
+					 * operations.  The Vars might be inside an expression,
+					 * such as a type coercion added by a parent set
+					 * operation.
 					 */
 					foreach(l, splan->plan.targetlist)
 					{
 						TargetEntry *tle = (TargetEntry *) lfirst(l);
 						Var		   *var = (Var *) tle->expr;
 
-						if (var && IsA(var, Var))
-						{
-							if (var->varno == ROWID_VAR)
-								tle->expr = (Expr *) makeNullConst(var->vartype,
-																   var->vartypmod,
-																   var->varcollid);
-							else if (var->varno == 0)
-								tle->expr = (Expr *) makeVar(1,
-															 var->varattno,
-															 var->vartype,
-															 var->vartypmod,
-															 var->varcollid,
-															 var->varlevelsup);
-						}
+						if (var && IsA(var, Var) && var->varno == ROWID_VAR)
+							tle->expr = (Expr *) makeNullConst(var->vartype,
+															   var->vartypmod,
+															   var->varcollid);
+						else
+							tle->expr = (Expr *)
+								fix_dummy_setop_vars_mutator((Node *) tle->expr,
+															 NULL);
 					}
 
 					splan->plan.targetlist =
@@ -2246,6 +2243,32 @@ fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan,
 	return (Node *) bestplan;
 }
 
+/*
+ * fix_dummy_setop_vars_mutator
+ *		Change the varno 0 Vars made by prepunion.c to varno 1.
+ */
+static Node *
+fix_dummy_setop_vars_mutator(Node *node, void *context)
+{
+	if (node == NULL)
+		return NULL;
+	if (IsA(node, Var))
+	{
+		Var		   *var = (Var *) node;
+
+		if (var->varno == 0)
+			return (Node *) makeVar(1,
+									var->varattno,
+									var->vartype,
+									var->vartypmod,
+									var->varcollid,
+									var->varlevelsup);
+		return node;
+	}
+	return expression_tree_mutator(node, fix_dummy_setop_vars_mutator,
+								   context);
+}
+
 /*
  * fix_scan_expr
  *		Do set_plan_references processing on a scan-level expression
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 84abcd6b14f..785fde1c8e3 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -1388,6 +1388,26 @@ SELECT ten FROM tenk1 dummy WHERE 1=2;
                      Output: t2.four
 (11 rows)
 
+-- Ensure EXPLAIN can show a dummy set operation whose output is coerced to
+-- another type by the parent set operation
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT two::numeric FROM tenk1
+EXCEPT
+(SELECT four FROM tenk1 WHERE 1=2
+ EXCEPT ALL
+ SELECT ten FROM tenk1);
+                             QUERY PLAN                              
+---------------------------------------------------------------------
+ HashSetOp Except
+   Output: ((tenk1.two)::numeric)
+   ->  Seq Scan on public.tenk1
+         Output: (tenk1.two)::numeric
+   ->  Result
+         Output: two
+         Replaces: Aggregate on unnamed_subquery, unnamed_subquery_1
+         One-Time Filter: false
+(8 rows)
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index c8de276c2b5..c1f349cabc0 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -531,6 +531,15 @@ SELECT four FROM tenk1 t2
 UNION
 SELECT ten FROM tenk1 dummy WHERE 1=2;
 
+-- Ensure EXPLAIN can show a dummy set operation whose output is coerced to
+-- another type by the parent set operation
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT two::numeric FROM tenk1
+EXCEPT
+(SELECT four FROM tenk1 WHERE 1=2
+ EXCEPT ALL
+ SELECT ten FROM tenk1);
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
-- 
2.37.1 (Apple Git-137.1)



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-05 00:36  Tom Lane <tgl@sss.pgh.pa.us>
  parent: shihao zhong <zhong950419@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2026-10-05 00:36 UTC (permalink / raw)
  To: shihao zhong <zhong950419@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

shihao zhong <zhong950419@gmail.com> writes:
> 928df067d1e handles a plain Var in the dummy Result's tlist.  Here
> the Var is under an int to numeric cast, so set_plan_refs() misses
> it.
> The attached patch walks the whole expression.  It keeps the
> varno 1 rewrite, so for a nested setop the name shown can come from
> another child, as in the new test's output.

Good catch, but there's another problem here.  I wondered why the
Result is claiming to output "two", when that is not either of
the columns being output by the removed setop leaf queries.
This same code is at fault: it's injecting varno "1" without
regard for which of the leaf queries are actually represented.
Fortunately, now that we have Result.relids, it's pretty easy
to discover which leaf queries are represented and choose the
leftmost one.  Hence, v2 attached.

By the way, I'm still not super happy about

+         Replaces: Aggregate on unnamed_subquery, unnamed_subquery_1

when there is no aggregation going on anywhere.  But that's
because Robert took shortcuts: show_result_replacement_info
does

		case RESULT_TYPE_UPPER:
			/* a small white lie */
			replacement_type = "Aggregate";
			break;

without regard for the actual reason the Result got injected.
I recall complaining about that and Robert not wanting to add
yet more complexity to what he was doing.  Which is fair,
but I still think we're gonna get bug reports about this.

			regards, tom lane

Attachments:

  [text/x-diff] v2-0001-Fix-EXPLAIN-of-dummy-set-operations-some-more.patch (7.5K, ../../598880.1791160576@sss.pgh.pa.us/2-v2-0001-Fix-EXPLAIN-of-dummy-set-operations-some-more.patch)
  download | inline diff:
From 40c46b9c5147780668da5670f62e94af3301a5a8 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 4 Oct 2026 20:26:31 -0400
Subject: [PATCH v2] Fix EXPLAIN of dummy set operations some more.

EXPLAIN failed to deal with varno-0 Vars that are made by prepunion.c
and can survive into a finished plan in the case where a provably
empty set operation is replaced by a dummy Result (which is possible
since 03d40e4b5).  Commit 928df067d tried to fix this, but it was a
couple bricks shy of a load.  First, it only dealt with varno-0 Vars
at the top level of the Result's tlist, but they could be buried
under coercion expressions.  Fix that by doing a recursive mutation.
Second, it always replaced varno 0 with varno 1, but that's just
wrong: the Result might represent a group of setop leaf queries that
do not include the leftmost leaf.  That led to displaying the wrong
variable(s) as outputs of the Result, risking confusion.  Fortunately,
we can get the actual child relids from the recently-added
Result.relids field, and use that to discover the leftmost child
represented by the Result.

This was found in discussion of bug #19742, but it's really an
independent issue.

Author: shihao zhong <zhong950419@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAGRkXqTFwygKmjLG_Y=kbXRHsePFmD6k+qrzVLP4_KrG+-=oRg@mail.gmail.com
Backpatch-through: 19
---
 src/backend/optimizer/plan/setrefs.c | 69 ++++++++++++++++++++--------
 src/test/regress/expected/union.out  | 20 ++++++++
 src/test/regress/sql/union.sql       |  9 ++++
 3 files changed, 79 insertions(+), 19 deletions(-)

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 8a641402a96..327c0febe89 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -155,6 +155,7 @@ static Plan *set_mergeappend_references(PlannerInfo *root,
 										int rtoffset);
 static void set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset);
 static Relids offset_relid_set(Relids relids, int rtoffset);
+static Node *fix_dummy_setop_vars_mutator(Node *node, int *first_child_relid);
 static Node *fix_scan_expr(PlannerInfo *root, Node *node,
 						   int rtoffset, double num_exec);
 static Node *fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context);
@@ -1041,6 +1042,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 					set_upper_references(root, plan, rtoffset);
 				else
 				{
+					int			first_child_relid;
+
 					/*
 					 * The tlist of a childless Result could contain
 					 * unresolved ROWID_VAR Vars, in case it's representing a
@@ -1054,33 +1057,35 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 					 * shouldn't be seen by fix_scan_expr.
 					 *
 					 * We also must handle the case where set operations have
-					 * been short-circuited resulting in a dummy Result node.
-					 * prepunion.c uses varno==0 for the set op targetlist.
-					 * See generate_setop_tlist() and generate_setop_tlist().
-					 * Here we rewrite these to use varno==1, which is the
-					 * varno of the first set-op child.  Without this, EXPLAIN
+					 * been proven empty, resulting in a dummy Result node.
+					 * Because prepunion.c uses varno 0 for setop targetlists,
+					 * that's what we'll find here.  Replace such Vars with
+					 * Vars pointing at the Result's lowest-numbered replaced
+					 * rel, which will be its leftmost set-op child.  While we
+					 * can assume that ROWID_VARs are at top level, varno 0
+					 * Vars might be buried in coercion expressions, so that
+					 * needs a recursive traversal.  Without this, EXPLAIN
 					 * will have trouble displaying targetlists of dummy set
 					 * operations.
+					 *
+					 * Note that some Results have empty relids, leading to
+					 * first_child_relid being negative.  We assume such
+					 * Results can't contain any varno 0 Vars.
 					 */
+					first_child_relid = bms_next_member(splan->relids, -1);
 					foreach(l, splan->plan.targetlist)
 					{
 						TargetEntry *tle = (TargetEntry *) lfirst(l);
 						Var		   *var = (Var *) tle->expr;
 
-						if (var && IsA(var, Var))
-						{
-							if (var->varno == ROWID_VAR)
-								tle->expr = (Expr *) makeNullConst(var->vartype,
-																   var->vartypmod,
-																   var->varcollid);
-							else if (var->varno == 0)
-								tle->expr = (Expr *) makeVar(1,
-															 var->varattno,
-															 var->vartype,
-															 var->vartypmod,
-															 var->varcollid,
-															 var->varlevelsup);
-						}
+						if (var && IsA(var, Var) && var->varno == ROWID_VAR)
+							tle->expr = (Expr *) makeNullConst(var->vartype,
+															   var->vartypmod,
+															   var->varcollid);
+						else if (first_child_relid > 0)
+							tle->expr = (Expr *)
+								fix_dummy_setop_vars_mutator((Node *) tle->expr,
+															 &first_child_relid);
 					}
 
 					splan->plan.targetlist =
@@ -2246,6 +2251,32 @@ fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan,
 	return (Node *) bestplan;
 }
 
+/*
+ * fix_dummy_setop_vars_mutator
+ *		Change the varno 0 Vars made by prepunion.c to varno *first_child_relid.
+ */
+static Node *
+fix_dummy_setop_vars_mutator(Node *node, int *first_child_relid)
+{
+	if (node == NULL)
+		return NULL;
+	if (IsA(node, Var))
+	{
+		Var		   *var = (Var *) node;
+
+		if (var->varno == 0)
+			return (Node *) makeVar(*first_child_relid,
+									var->varattno,
+									var->vartype,
+									var->vartypmod,
+									var->varcollid,
+									var->varlevelsup);
+		return node;
+	}
+	return expression_tree_mutator(node, fix_dummy_setop_vars_mutator,
+								   first_child_relid);
+}
+
 /*
  * fix_scan_expr
  *		Do set_plan_references processing on a scan-level expression
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 84abcd6b14f..f07b6141e75 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -1388,6 +1388,26 @@ SELECT ten FROM tenk1 dummy WHERE 1=2;
                      Output: t2.four
 (11 rows)
 
+-- Ensure EXPLAIN can show a dummy set operation whose output is coerced
+-- to another type by the parent set operation.
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT two::numeric FROM tenk1 t1
+EXCEPT
+(SELECT four FROM tenk1 dummy WHERE 1=2
+ EXCEPT ALL
+ SELECT ten FROM tenk1 t2);
+                             QUERY PLAN                              
+---------------------------------------------------------------------
+ HashSetOp Except
+   Output: ((t1.two)::numeric)
+   ->  Seq Scan on public.tenk1 t1
+         Output: (t1.two)::numeric
+   ->  Result
+         Output: unnamed_subquery.four
+         Replaces: Aggregate on unnamed_subquery, unnamed_subquery_1
+         One-Time Filter: false
+(8 rows)
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index c8de276c2b5..a787dd25d23 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -531,6 +531,15 @@ SELECT four FROM tenk1 t2
 UNION
 SELECT ten FROM tenk1 dummy WHERE 1=2;
 
+-- Ensure EXPLAIN can show a dummy set operation whose output is coerced
+-- to another type by the parent set operation.
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT two::numeric FROM tenk1 t1
+EXCEPT
+(SELECT four FROM tenk1 dummy WHERE 1=2
+ EXCEPT ALL
+ SELECT ten FROM tenk1 t2);
+
 -- Test constraint exclusion of UNION ALL subqueries
 explain (costs off)
  SELECT * FROM
-- 
2.52.0

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-05 02:20  shihao zhong <zhong950419@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 14+ messages in thread

From: shihao zhong @ 2026-10-05 02:20 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: David Rowley <dgrowleyml@gmail.com>; feasiblechart@gmail.com; pgsql-bugs@lists.postgresql.org

Hi Tom,
> Fortunately, now that we have Result.relids, it's pretty easy
> to discover which leaf queries are represented and choose the
> leftmost one.  Hence, v2 attached.

Thanks, v2 looks good to me.  With it, EXPLAIN works on all of the
same 20000 random queries.

With the dummy setop inside a subquery, under a JOIN or  GROUP BY,
the Result shows the columns of its own left-most leaf.

Thanks,
Shihao

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-05 17:39  Tom Lane <tgl@sss.pgh.pa.us>
  parent: shihao zhong <zhong950419@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2026-10-05 17:39 UTC (permalink / raw)
  To: shihao zhong <zhong950419@gmail.com>; +Cc: feasiblechart@gmail.com, David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org

shihao zhong <zhong950419@gmail.com> writes:
> This is the small fix, meant for 19 and master.

Sorry for having overlooked this email earlier.  It's substantially
the same fix I came up with, so I credited you as co-author.
I've pushed these two fixes and marked the open item as done.

> I think a better fix would make the pathkeys right in both places,
> so the Append can keep them.  I can work on that for master if you
> like.

If you're interested in working on a long-term fix, I think the path
forward ought to be to get rid of these "varno 0" Vars in favor of
using ordinary Vars that reference real RangeTblEntrys.  Right now,
a Query level that represents a set-op nest only has RTE_SUBQUERY
RTEs for the leaf queries.  I'm imagining inventing a new RTEKind,
say RTE_SETOP, and building one of those for each set operation
in the nest.  Then the Vars representing the output columns of that
set operation could carry that RTE's index, and everything gets a
lot less magic.  I'm not sure that there would be any large reduction
in total lines of code, but it'd be cleaner, and there are some things
such as tlist width estimation that would work better.

While we could move much of what's in SetOperationStmt into such
RTEs, I'd be inclined not to, because additional fields in
RangeTblEntry would just be bloat for non-SETOP RTEs.  So my
druthers would be to add no new fields to RangeTblEntry, just
re-use whatever ones are there that are useful.  SetOperationStmt
probably needs to gain a field for the index of the associated
RTE, though.

IIRC, there are XXX comments in prepunion.c whining about how
building the setop tlists ought to be done at parse time, so
that's something we could look into at the same time, or as a
follow-on patch.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t"
@ 2026-10-05 18:39  shihao zhong <zhong950419@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 14+ messages in thread

From: shihao zhong @ 2026-10-05 18:39 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: feasiblechart@gmail.com, David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org

Hi Tom,

Thanks for pushing these, and for the credit.

> I'm imagining inventing a new RTEKind, say RTE_SETOP, and building
> one of those for each set operation in the nest.

Yes, I would like to work on that. I will start with the RTE and the
Vars, and leave building the setop tlists at parse time for a
follow-on patch.

I will post the first version in a new thread on hackers.

Thanks,
Shihao

^ permalink  raw  reply  [nested|flat] 14+ messages in thread


end of thread, other threads:[~2026-10-05 18:39 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-10-03 16:17 BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t" PG Bug reporting form <noreply@postgresql.org>
2026-10-03 21:53 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-04 01:00   ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-04 05:37     ` David Rowley <dgrowleyml@gmail.com>
2026-10-04 05:42       ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-04 17:01         ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-04 18:41           ` shihao zhong <zhong950419@gmail.com>
2026-10-04 19:39             ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-04 21:43               ` shihao zhong <zhong950419@gmail.com>
2026-10-05 00:36                 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-05 02:20                   ` shihao zhong <zhong950419@gmail.com>
2026-10-04 05:23   ` shihao zhong <zhong950419@gmail.com>
2026-10-05 17:39     ` Tom Lane <tgl@sss.pgh.pa.us>
2026-10-05 18:39       ` shihao zhong <zhong950419@gmail.com>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox