From fca8e59c1c582030dd7a3c870e1c3c70e8a193aa Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 26 Jul 2015 17:44:27 -0400 Subject: [PATCH] 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. --- src/backend/optimizer/prep/prepjointree.c | 52 +++++++++++++++-------- src/test/regress/expected/join.out | 34 +++++++++++++++ src/test/regress/sql/join.sql | 13 ++++++ 3 files changed, 81 insertions(+), 18 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 34144ccaf0..9bf1c662b5 100644 --- 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 96ec997ed1..4ce01cbcd5 100644 --- 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 ada78db264..3a71dbf4df 100644 --- 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 -- -- GitLab