Fix oversight in flattening of subqueries with empty FROM.

I missed a restriction that commit f4abd0241de20d5d6a79b84992b9e88603d44134
should have enforced: we can't pull up an empty-FROM subquery if it's under
an outer join, because then we'd need to wrap its output columns in
PlaceHolderVars.  As the code currently stands, the PHVs end up with empty
relid sets, which doesn't work (and is correctly caught by an Assert).

It's possible that this could be fixed by assigning the PHVs the relid
sets of the parent FromExpr/JoinExpr, but getting that to work is more
complication than I care to add right now; indeed it's likely that
we'll never bother, since pulling up empty-FROM subqueries is a rather
marginal optimization anyway.

Per report from Andreas Seltenreich.  Back-patch to 9.5 where the faulty
code was added.

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 34144ccaf0fa69161541bf5785d9d20d9a162cda..9bf1c662b5371614dabb5b39dd90a5f676f3678f 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -1435,25 +1435,40 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte,
 
        /*
         * Don't pull up a subquery with an empty jointree, unless it has no quals
-        * and deletion_ok is TRUE.  query_planner() will correctly generate a
-        * Result plan for a jointree that's totally empty, but we can't cope with
-        * an empty FromExpr appearing lower down in a jointree: we identify join
-        * rels via baserelid sets, so we couldn't distinguish a join containing
-        * such a FromExpr from one without it.  This would for example break the
-        * PlaceHolderVar mechanism, since we'd have no way to identify where to
-        * evaluate a PHV coming out of the subquery.  We can only handle such
-        * cases if the place where the subquery is linked is a FromExpr or inner
-        * JOIN that would still be nonempty after removal of the subquery, so
-        * that it's still identifiable via its contained baserelids.  Safe
-        * contexts are signaled by deletion_ok.  But even in a safe context, we
-        * must keep the subquery if it has any quals, because it's unclear where
-        * to put them in the upper query.  (Note that deletion of a subquery is
-        * also dependent on the check below that its targetlist contains no
-        * set-returning functions.  Deletion from a FROM list or inner JOIN is
-        * okay only if the subquery must return exactly one row.)
+        * and deletion_ok is TRUE and we're not underneath an outer join.
+        *
+        * query_planner() will correctly generate a Result plan for a jointree
+        * that's totally empty, but we can't cope with an empty FromExpr
+        * appearing lower down in a jointree: we identify join rels via baserelid
+        * sets, so we couldn't distinguish a join containing such a FromExpr from
+        * one without it.  We can only handle such cases if the place where the
+        * subquery is linked is a FromExpr or inner JOIN that would still be
+        * nonempty after removal of the subquery, so that it's still identifiable
+        * via its contained baserelids.  Safe contexts are signaled by
+        * deletion_ok.
+        *
+        * But even in a safe context, we must keep the subquery if it has any
+        * quals, because it's unclear where to put them in the upper query.
+        *
+        * Also, we must forbid pullup if such a subquery is underneath an outer
+        * join, because then we might need to wrap its output columns with
+        * PlaceHolderVars, and the PHVs would then have empty relid sets meaning
+        * we couldn't tell where to evaluate them.  (This test is separate from
+        * the deletion_ok flag for possible future expansion: deletion_ok tells
+        * whether the immediate parent site in the jointree could cope, not
+        * whether we'd have PHV issues.  It's possible this restriction could be
+        * fixed by letting the PHVs use the relids of the parent jointree item,
+        * but that complication is for another day.)
+        *
+        * Note that deletion of a subquery is also dependent on the check below
+        * that its targetlist contains no set-returning functions.  Deletion from
+        * a FROM list or inner JOIN is okay only if the subquery must return
+        * exactly one row.
         */
        if (subquery->jointree->fromlist == NIL &&
-               (subquery->jointree->quals || !deletion_ok))
+               (subquery->jointree->quals != NULL ||
+                !deletion_ok ||
+                lowest_outer_join != NULL))
                return false;
 
        /*
@@ -1667,7 +1682,8 @@ is_simple_values(PlannerInfo *root, RangeTblEntry *rte, bool deletion_ok)
 
        /*
         * Because VALUES can't appear under an outer join (or at least, we won't
-        * try to pull it up if it does), we need not worry about LATERAL.
+        * try to pull it up if it does), we need not worry about LATERAL, nor
+        * about validity of PHVs for the VALUES' outputs.
         */
 
        /*

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 96ec997ed1661005a5a470ffd19eed3653cdb2ec..4ce01cbcd5b6b7417150480facb0294ff3e2d52f 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2184,6 +2184,40 @@ select aa, bb, unique1, unique1
 ----+----+---------+---------
 (0 rows)
 
+--
+-- regression test: check handling of empty-FROM subquery underneath outer join
+--
+explain (costs off)
+select * from int8_tbl i1 left join (int8_tbl i2 join
+  (select 123 as x) ss on i2.q1 = x) on i1.q2 = i2.q2
+order by 1, 2;
+                   QUERY PLAN                    
+-------------------------------------------------
+ Sort
+   Sort Key: i1.q1, i1.q2
+   ->  Hash Left Join
+         Hash Cond: (i1.q2 = i2.q2)
+         ->  Seq Scan on int8_tbl i1
+         ->  Hash
+               ->  Hash Join
+                     Hash Cond: (i2.q1 = (123))
+                     ->  Seq Scan on int8_tbl i2
+                     ->  Hash
+                           ->  Result
+(11 rows)
+
+select * from int8_tbl i1 left join (int8_tbl i2 join
+  (select 123 as x) ss on i2.q1 = x) on i1.q2 = i2.q2
+order by 1, 2;
+        q1        |        q2         | q1  |        q2        |  x  
+------------------+-------------------+-----+------------------+-----
+              123 |               456 | 123 |              456 | 123
+              123 |  4567890123456789 | 123 | 4567890123456789 | 123
+ 4567890123456789 | -4567890123456789 |     |                  |    
+ 4567890123456789 |               123 |     |                  |    
+ 4567890123456789 |  4567890123456789 | 123 | 4567890123456789 | 123
+(5 rows)
+
 --
 -- Clean up
 --

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index ada78db2644707cd94e79fab68a1696708d522d2..3a71dbf4dffe38078ab5f204014cb65abd57a81b 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -365,6 +365,19 @@ select aa, bb, unique1, unique1
   from tenk1 right join b on aa = unique1
   where bb < bb and bb is null;
 
+--
+-- regression test: check handling of empty-FROM subquery underneath outer join
+--
+explain (costs off)
+select * from int8_tbl i1 left join (int8_tbl i2 join
+  (select 123 as x) ss on i2.q1 = x) on i1.q2 = i2.q2
+order by 1, 2;
+
+select * from int8_tbl i1 left join (int8_tbl i2 join
+  (select 123 as x) ss on i2.q1 = x) on i1.q2 = i2.q2
+order by 1, 2;
+
+
 --
 -- Clean up
 --