diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 5482ab85a7..4195a0a84f 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -2490,14 +2490,48 @@ pullup_replace_vars_callback(Var *var, else wrap = false; } + else if (rcon->wrap_non_vars) + { + /* Caller told us to wrap all non-Vars in a PlaceHolderVar */ + wrap = true; + } else { /* - * Must wrap, either because we need a place to insert - * varnullingrels or because caller told us to wrap - * everything. + * If the node contains Var(s) or PlaceHolderVar(s) of the + * subquery being pulled up, and does not contain any + * non-strict constructs, then instead of adding a PHV on top + * we can add the required nullingrels to those Vars/PHVs. + * (This is fundamentally a generalization of the above cases + * for bare Vars and PHVs.) + * + * This test is somewhat expensive, but it avoids pessimizing + * the plan in cases where the nullingrels get removed again + * later by outer join reduction. + * + * This analysis could be tighter: in particular, a non-strict + * construct hidden within a lower-level PlaceHolderVar is not + * reason to add another PHV. But for now it doesn't seem + * worth the code to be more exact. + * + * For a LATERAL subquery, we have to check the actual var + * membership of the node, but if it's non-lateral then any + * level-zero var must belong to the subquery. */ - wrap = true; + if ((rcon->target_rte->lateral ? + bms_overlap(pull_varnos(rcon->root, newnode), + rcon->relids) : + contain_vars_of_level(newnode, 0)) && + !contain_nonstrict_functions(newnode)) + { + /* No wrap needed */ + wrap = false; + } + else + { + /* Else wrap it in a PlaceHolderVar */ + wrap = true; + } } if (wrap) @@ -2518,18 +2552,14 @@ pullup_replace_vars_callback(Var *var, } } - /* Must adjust varlevelsup if replaced Var is within a subquery */ - if (var->varlevelsup > 0) - IncrementVarSublevelsUp(newnode, var->varlevelsup, 0); - - /* Propagate any varnullingrels into the replacement Var or PHV */ + /* Propagate any varnullingrels into the replacement expression */ if (var->varnullingrels != NULL) { if (IsA(newnode, Var)) { Var *newvar = (Var *) newnode; - Assert(newvar->varlevelsup == var->varlevelsup); + Assert(newvar->varlevelsup == 0); newvar->varnullingrels = bms_add_members(newvar->varnullingrels, var->varnullingrels); } @@ -2537,14 +2567,26 @@ pullup_replace_vars_callback(Var *var, { PlaceHolderVar *newphv = (PlaceHolderVar *) newnode; - Assert(newphv->phlevelsup == var->varlevelsup); + Assert(newphv->phlevelsup == 0); newphv->phnullingrels = bms_add_members(newphv->phnullingrels, var->varnullingrels); } else - elog(ERROR, "failed to wrap a non-Var"); + { + /* There should be lower-level Vars/PHVs we can modify */ + newnode = add_nulling_relids(newnode, + NULL, /* modify all Vars/PHVs */ + var->varnullingrels); + /* Assert we did put the varnullingrels into the expression */ + Assert(bms_is_subset(var->varnullingrels, + pull_varnos(rcon->root, newnode))); + } } + /* Must adjust varlevelsup if replaced Var is within a subquery */ + if (var->varlevelsup > 0) + IncrementVarSublevelsUp(newnode, var->varlevelsup, 0); + return newnode; } diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 191f2dc0b1..b20625fbd2 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -1141,7 +1141,8 @@ AddInvertedQual(Query *parsetree, Node *qual) /* * add_nulling_relids() finds Vars and PlaceHolderVars that belong to any * of the target_relids, and adds added_relids to their varnullingrels - * and phnullingrels fields. + * and phnullingrels fields. If target_relids is NULL, all level-zero + * Vars and PHVs are modified. */ Node * add_nulling_relids(Node *node, @@ -1170,7 +1171,8 @@ add_nulling_relids_mutator(Node *node, Var *var = (Var *) node; if (var->varlevelsup == context->sublevels_up && - bms_is_member(var->varno, context->target_relids)) + (context->target_relids == NULL || + bms_is_member(var->varno, context->target_relids))) { Relids newnullingrels = bms_union(var->varnullingrels, context->added_relids); @@ -1188,7 +1190,8 @@ add_nulling_relids_mutator(Node *node, PlaceHolderVar *phv = (PlaceHolderVar *) node; if (phv->phlevelsup == context->sublevels_up && - bms_overlap(phv->phrels, context->target_relids)) + (context->target_relids == NULL || + bms_overlap(phv->phrels, context->target_relids))) { Relids newnullingrels = bms_union(phv->phnullingrels, context->added_relids); diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 9eecdc1e92..2d35de3fad 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -1721,6 +1721,35 @@ fetch backward all in c1; (2 rows) commit; +-- +-- Verify that we correctly flatten cases involving a subquery output +-- expression that doesn't need to be wrapped in a PlaceHolderVar +-- +explain (costs off) +select tname, attname from ( +select relname::information_schema.sql_identifier as tname, * from + (select * from pg_class c) ss1) ss2 + right join pg_attribute a on a.attrelid = ss2.oid +where tname = 'tenk1' and attnum = 1; + QUERY PLAN +-------------------------------------------------------------------------- + Nested Loop + -> Index Scan using pg_class_relname_nsp_index on pg_class c + Index Cond: (relname = 'tenk1'::name) + -> Index Scan using pg_attribute_relid_attnum_index on pg_attribute a + Index Cond: ((attrelid = c.oid) AND (attnum = 1)) +(5 rows) + +select tname, attname from ( +select relname::information_schema.sql_identifier as tname, * from + (select * from pg_class c) ss1) ss2 + right join pg_attribute a on a.attrelid = ss2.oid +where tname = 'tenk1' and attnum = 1; + tname | attname +-------+--------- + tenk1 | unique1 +(1 row) + -- -- Tests for CTE inlining behavior -- diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 75a9b718b2..af6e157aca 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -890,6 +890,24 @@ fetch backward all in c1; commit; +-- +-- Verify that we correctly flatten cases involving a subquery output +-- expression that doesn't need to be wrapped in a PlaceHolderVar +-- + +explain (costs off) +select tname, attname from ( +select relname::information_schema.sql_identifier as tname, * from + (select * from pg_class c) ss1) ss2 + right join pg_attribute a on a.attrelid = ss2.oid +where tname = 'tenk1' and attnum = 1; + +select tname, attname from ( +select relname::information_schema.sql_identifier as tname, * from + (select * from pg_class c) ss1) ss2 + right join pg_attribute a on a.attrelid = ss2.oid +where tname = 'tenk1' and attnum = 1; + -- -- Tests for CTE inlining behavior --