Flatten join alias Vars before pulling up targetlist items from a subquery.

pullup_replace_vars()'s decisions about whether a pulled-up replacement
expression needs to be wrapped in a PlaceHolderVar depend on the assumption
that what looks like a Var behaves like a Var.  However, if the Var is a
join alias reference, later flattening of join aliases might replace the
Var with something that's not a Var at all, and should have been wrapped.

To fix, do a forcible pass of flatten_join_alias_vars() on the subquery
targetlist before we start to copy items out of it.  We'll re-run that
processing on the pulled-up expressions later, but that's harmless.

Per report from Ken Tanzer; the added regression test case is based on his
example.  This bug has been there since the PlaceHolderVar mechanism was
invented, but has escaped detection because the circumstances that trigger
it are fairly narrow.  You need a flattenable query underneath an outer
join, which contains another flattenable query inside a join of its own,
with a dangerous expression (a constant or something else non-strict)
in that one's targetlist.

Having seen this, I'm wondering if it wouldn't be prudent to do all
alias-variable flattening earlier, perhaps even in the rewriter.
But that would probably not be a back-patchable change.

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 639f0369dee2427bd98f8ff72a9ca1b9a791a634..3e9977652261d0ead43e55c9e3e4b6e8ee9a07ab 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -728,6 +728,18 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
                return jtnode;
        }
 
+       /*
+        * We must flatten any join alias Vars in the subquery's targetlist,
+        * because pulling up the subquery's subqueries might have changed their
+        * expansions into arbitrary expressions, which could affect
+        * pullup_replace_vars' decisions about whether PlaceHolderVar wrappers
+        * are needed for tlist entries.  (Likely it'd be better to do
+        * flatten_join_alias_vars on the whole query tree at some earlier stage,
+        * maybe even in the rewriter; but for now let's just fix this case here.)
+        */
+       subquery->targetList = (List *)
+               flatten_join_alias_vars(subroot, (Node *) subquery->targetList);
+
        /*
         * Adjust level-0 varnos in subquery so that we can append its rangetable
         * to upper query's.  We have to fix the subquery's append_rel_list as

diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 5b41d6320f58a97cf837f8b725608cc25af0827e..705bf243d92c63f85f136d59783308ecd99a9c6b 100644 (file)
--- a/src/backend/optimizer/util/var.c
+++ b/src/backend/optimizer/util/var.c
@@ -750,16 +750,14 @@ flatten_join_alias_vars_mutator(Node *node,
                                /* Ignore dropped columns */
                                if (newvar == NULL)
                                        continue;
+                               newvar = copyObject(newvar);
 
                                /*
                                 * If we are expanding an alias carried down from an upper
                                 * query, must adjust its varlevelsup fields.
                                 */
                                if (context->sublevels_up != 0)
-                               {
-                                       newvar = copyObject(newvar);
                                        IncrementVarSublevelsUp(newvar, context->sublevels_up, 0);
-                               }
                                /* Recurse in case join input is itself a join */
                                /* (also takes care of setting inserted_sublink if needed) */
                                newvar = flatten_join_alias_vars_mutator(newvar, context);
@@ -779,16 +777,14 @@ flatten_join_alias_vars_mutator(Node *node,
                Assert(var->varattno > 0);
                newvar = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
                Assert(newvar != NULL);
+               newvar = copyObject(newvar);
 
                /*
                 * If we are expanding an alias carried down from an upper query, must
                 * adjust its varlevelsup fields.
                 */
                if (context->sublevels_up != 0)
-               {
-                       newvar = copyObject(newvar);
                        IncrementVarSublevelsUp(newvar, context->sublevels_up, 0);
-               }
 
                /* Recurse in case join input is itself a join */
                newvar = flatten_join_alias_vars_mutator(newvar, context);

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index dabb1857e30bfac61e1de6092d836668678441ce..2b04945051d9edef95388c4bdd0a596b4b60f320 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2595,3 +2595,24 @@ select f1, unique2, case when unique2 is null then f1 else 0 end
   0 |       0 |    0
 (1 row)
 
+--
+-- check handling of join aliases when flattening multiple levels of subquery
+--
+select foo1.join_key as foo1_id, foo3.join_key AS foo3_id, bug_field from
+  (values (0),(1)) foo1(join_key)
+left join
+  (select join_key, bug_field from
+    (select ss1.join_key, ss1.bug_field from
+      (select f1 as join_key, 666 as bug_field from int4_tbl i1) ss1
+    ) foo2
+   left join
+    (select unique2 as join_key from tenk1 i2) ss2
+   using (join_key)
+  ) foo3
+using (join_key);
+ foo1_id | foo3_id | bug_field 
+---------+---------+-----------
+       0 |       0 |       666
+       1 |         |          
+(2 rows)
+

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 1d7396dbaf797334b780351eae35b3a8ff91e9e8..41def9c7a0fe93daa5f65074fba88a55c5fad9e0 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -651,3 +651,20 @@ select q1, unique2, thousand, hundred
 select f1, unique2, case when unique2 is null then f1 else 0 end
   from int4_tbl a left join tenk1 b on f1 = unique2
   where (case when unique2 is null then f1 else 0 end) = 0;
+
+--
+-- check handling of join aliases when flattening multiple levels of subquery
+--
+
+select foo1.join_key as foo1_id, foo3.join_key AS foo3_id, bug_field from
+  (values (0),(1)) foo1(join_key)
+left join
+  (select join_key, bug_field from
+    (select ss1.join_key, ss1.bug_field from
+      (select f1 as join_key, 666 as bug_field from int4_tbl i1) ss1
+    ) foo2
+   left join
+    (select unique2 as join_key from tenk1 i2) ss2
+   using (join_key)
+  ) foo3
+using (join_key);