Re: BUG #19742: `INTERSECT` under a `UNION ALL` with an empty arm fails with "could not find pathkey item t" - Mailing list pgsql-bugs

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

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