Fix pull_varnos to cope with translated PlaceHolderVars.

Commit 55dc86eca changed pull_varnos to use (if possible) the associated
ph_eval_at for a PlaceHolderVar.  I missed a fine point though: we might
be looking at a PHV in the quals or tlist of a child appendrel, in which
case we need to compute a ph_eval_at value that's been translated in the
same way that the PHV itself has been (cf. adjust_appendrel_attrs).
Fortunately, enough info is available in the PlaceHolderInfo to make
such translation possible without additional outside data, so we don't
need another round of uglification of planner APIs.  This is a little
bit complicated, but since it's a hard-to-hit corner case, I'm not much
worried about adding cycles here.

Per report from Jaime Casanova.  Back-patch to v12, like the previous
commit.

Discussion: https://postgr.es/m/20210915230959.GB17635@ahch-to

diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 58d093c1c1105b81c05cc3a8c6d18f8d7ac09909..101b56d03f8d447cbc9e4eecfd4e108ffdb6c028 100644 (file)
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -200,6 +200,16 @@ pull_varnos_walker(Node *node, pull_varnos_context *context)
             * join that forces delay of evaluation of a given qual clause
             * will be processed before we examine that clause here, so the
             * ph_eval_at value should have been updated to include it.
+            *
+            * Another problem is that a PlaceHolderVar can appear in quals or
+            * tlists that have been translated for use in a child appendrel.
+            * Typically such a PHV is a parameter expression sourced by some
+            * other relation, so that the translation from parent appendrel
+            * to child doesn't change its phrels, and we should still take
+            * ph_eval_at at face value.  But in corner cases, the PHV's
+            * original phrels can include the parent appendrel itself, in
+            * which case the translated PHV will have the child appendrel in
+            * phrels, and we must translate ph_eval_at to match.
             */
            PlaceHolderInfo *phinfo = NULL;
 
@@ -215,12 +225,37 @@ pull_varnos_walker(Node *node, pull_varnos_context *context)
                    phinfo = NULL;
                }
            }
-           if (phinfo != NULL)
+           if (phinfo == NULL)
+           {
+               /* No PlaceHolderInfo yet, use phrels */
+               context->varnos = bms_add_members(context->varnos,
+                                                 phv->phrels);
+           }
+           else if (bms_equal(phv->phrels, phinfo->ph_var->phrels))
+           {
+               /* Normal case: use ph_eval_at */
                context->varnos = bms_add_members(context->varnos,
                                                  phinfo->ph_eval_at);
+           }
            else
-               context->varnos = bms_add_members(context->varnos,
-                                                 phv->phrels);
+           {
+               /* Translated PlaceHolderVar: translate ph_eval_at to match */
+               Relids      newevalat,
+                           delta;
+
+               /* remove what was removed from phv->phrels ... */
+               delta = bms_difference(phinfo->ph_var->phrels, phv->phrels);
+               newevalat = bms_difference(phinfo->ph_eval_at, delta);
+               /* ... then if that was in fact part of ph_eval_at ... */
+               if (!bms_equal(newevalat, phinfo->ph_eval_at))
+               {
+                   /* ... add what was added */
+                   delta = bms_difference(phv->phrels, phinfo->ph_var->phrels);
+                   newevalat = bms_join(newevalat, delta);
+               }
+               context->varnos = bms_join(context->varnos,
+                                          newevalat);
+           }
            return false;       /* don't recurse into expression */
        }
    }

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 7154909632226dcb45c84fea733d8c5535aac802..772028748be6cce77e9eec77e3a277c6d305d545 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -4629,6 +4629,34 @@ where q2 = 456;
  123 | 456 |   |        
 (1 row)
 
+-- and check a related issue where we miscompute required relids for
+-- a PHV that's been translated to a child rel
+create temp table parttbl (a integer primary key) partition by range (a);
+create temp table parttbl1 partition of parttbl for values from (1) to (100);
+insert into parttbl values (11), (12);
+explain (costs off)
+select * from
+  (select *, 12 as phv from parttbl) as ss
+  right join int4_tbl on true
+where ss.a = ss.phv and f1 = 0;
+         QUERY PLAN         
+----------------------------
+ Nested Loop
+   ->  Seq Scan on int4_tbl
+         Filter: (f1 = 0)
+   ->  Seq Scan on parttbl1
+         Filter: (a = 12)
+(5 rows)
+
+select * from
+  (select *, 12 as phv from parttbl) as ss
+  right join int4_tbl on true
+where ss.a = ss.phv and f1 = 0;
+ a  | phv | f1 
+----+-----+----
+ 12 |  12 |  0
+(1 row)
+
 -- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
 select * from
   int8_tbl x join (int4_tbl x cross join int4_tbl y) j on q1 = f1; -- error

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 7af22b9f67a94e9784165f8b307ff7b7a5e48139..fde993a17021548775e7f2c10656f49a4f710814 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1635,6 +1635,22 @@ select i8.*, ss.v, t.unique2
     left join tenk1 t on t.unique2 = ss.v
 where q2 = 456;
 
+-- and check a related issue where we miscompute required relids for
+-- a PHV that's been translated to a child rel
+create temp table parttbl (a integer primary key) partition by range (a);
+create temp table parttbl1 partition of parttbl for values from (1) to (100);
+insert into parttbl values (11), (12);
+explain (costs off)
+select * from
+  (select *, 12 as phv from parttbl) as ss
+  right join int4_tbl on true
+where ss.a = ss.phv and f1 = 0;
+
+select * from
+  (select *, 12 as phv from parttbl) as ss
+  right join int4_tbl on true
+where ss.a = ss.phv and f1 = 0;
+
 -- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
 
 select * from