diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index dad859aedc69a8ab2e476d3f01c63750bfb3f5f2..36e9992de69ade8047af24bd08daf03c572ea3fc 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -759,15 +759,19 @@ _equalPlaceHolderVar(const PlaceHolderVar *a, const PlaceHolderVar *b) /* * We intentionally do not compare phexpr. Two PlaceHolderVars with the * same ID and levelsup should be considered equal even if the contained - * expressions have managed to mutate to different states. One way in - * which that can happen is that initplan sublinks would get replaced by - * differently-numbered Params when sublink folding is done. (The end - * result of such a situation would be some unreferenced initplans, which - * is annoying but not really a problem.) + * expressions have managed to mutate to different states. This will + * happen during final plan construction when there are nested PHVs, since + * the inner PHV will get replaced by a Param in some copies of the outer + * PHV. Another way in which it can happen is that initplan sublinks + * could get replaced by differently-numbered Params when sublink folding + * is done. (The end result of such a situation would be some + * unreferenced initplans, which is annoying but not really a problem.) On + * the same reasoning, there is no need to examine phrels. * * COMPARE_NODE_FIELD(phexpr); + * + * COMPARE_BITMAPSET_FIELD(phrels); */ - COMPARE_BITMAPSET_FIELD(phrels); COMPARE_SCALAR_FIELD(phid); COMPARE_SCALAR_FIELD(phlevelsup); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 36997be5889fb19db6ed49c5665e33c373130065..c5df505d49196c164f4de5117f8c81405a709f67 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2532,16 +2532,37 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root) Assert(phv->phlevelsup == 0); /* - * If not to be replaced, just return the PlaceHolderVar unmodified. - * We use bms_overlap as a cheap/quick test to see if the PHV might be - * evaluated in the outer rels, and then grab its PlaceHolderInfo to - * tell for sure. + * Check whether we need to replace the PHV. We use bms_overlap as a + * cheap/quick test to see if the PHV might be evaluated in the outer + * rels, and then grab its PlaceHolderInfo to tell for sure. */ - if (!bms_overlap(phv->phrels, root->curOuterRels)) - return node; - if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at, - root->curOuterRels)) - return node; + if (!bms_overlap(phv->phrels, root->curOuterRels) || + !bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at, + root->curOuterRels)) + { + /* + * We can't replace the whole PHV, but we might still need to + * replace Vars or PHVs within its expression, in case it ends up + * actually getting evaluated here. (It might get evaluated in + * this plan node, or some child node; in the latter case we don't + * really need to process the expression here, but we haven't got + * enough info to tell if that's the case.) Flat-copy the PHV + * node and then recurse on its expression. + * + * Note that after doing this, we might have different + * representations of the contents of the same PHV in different + * parts of the plan tree. This is OK because equal() will just + * match on phid/phlevelsup, so setrefs.c will still recognize an + * upper-level reference to a lower-level copy of the same PHV. + */ + PlaceHolderVar *newphv = makeNode(PlaceHolderVar); + + memcpy(newphv, phv, sizeof(PlaceHolderVar)); + newphv->phexpr = (Expr *) + replace_nestloop_params_mutator((Node *) phv->phexpr, + root); + return (Node *) newphv; + } /* Create a Param representing the PlaceHolderVar */ param = assign_nestloop_param_placeholdervar(root, phv); /* Is this param already listed in root->curOuterParams? */ diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index e292ef05f6b05e182cff4dfab8fb43fc9961eb16..7b550c31bab48bd0e57eafbc0f0d63d857d83f5c 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2570,6 +2570,80 @@ SELECT qq, unique1 456 | 7318 (3 rows) +-- +-- nested nestloops can require nested PlaceHolderVars +-- +create temp table nt1 ( + id int primary key, + a1 boolean, + a2 boolean +); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "nt1_pkey" for table "nt1" +create temp table nt2 ( + id int primary key, + nt1_id int, + b1 boolean, + b2 boolean, + foreign key (nt1_id) references nt1(id) +); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "nt2_pkey" for table "nt2" +create temp table nt3 ( + id int primary key, + nt2_id int, + c1 boolean, + foreign key (nt2_id) references nt2(id) +); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "nt3_pkey" for table "nt3" +insert into nt1 values (1,true,true); +insert into nt1 values (2,true,false); +insert into nt1 values (3,false,false); +insert into nt2 values (1,1,true,true); +insert into nt2 values (2,2,true,false); +insert into nt2 values (3,3,false,false); +insert into nt3 values (1,1,true); +insert into nt3 values (2,2,false); +insert into nt3 values (3,3,true); +explain (costs off) +select nt3.id +from nt3 as nt3 + left join + (select nt2.*, (nt2.b1 and ss1.a3) AS b3 + from nt2 as nt2 + left join + (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1 + on ss1.id = nt2.nt1_id + ) as ss2 + on ss2.id = nt3.nt2_id +where nt3.id = 1 and ss2.b3; + QUERY PLAN +----------------------------------------------- + Nested Loop + -> Nested Loop + -> Index Scan using nt3_pkey on nt3 + Index Cond: (id = 1) + -> Index Scan using nt2_pkey on nt2 + Index Cond: (id = nt3.nt2_id) + -> Index Only Scan using nt1_pkey on nt1 + Index Cond: (id = nt2.nt1_id) + Filter: (nt2.b1 AND (id IS NOT NULL)) +(9 rows) + +select nt3.id +from nt3 as nt3 + left join + (select nt2.*, (nt2.b1 and ss1.a3) AS b3 + from nt2 as nt2 + left join + (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1 + on ss1.id = nt2.nt1_id + ) as ss2 + on ss2.id = nt3.nt2_id +where nt3.id = 1 and ss2.b3; + id +---- + 1 +(1 row) + -- -- test case where a PlaceHolderVar is propagated into a subquery -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 316a6e93d719180b653af945d03504225b8ee31c..5834d3dfdcce510d4b1823ade2538f33243bdc26 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -660,6 +660,64 @@ SELECT qq, unique1 USING (qq) INNER JOIN tenk1 c ON qq = unique2; +-- +-- nested nestloops can require nested PlaceHolderVars +-- + +create temp table nt1 ( + id int primary key, + a1 boolean, + a2 boolean +); +create temp table nt2 ( + id int primary key, + nt1_id int, + b1 boolean, + b2 boolean, + foreign key (nt1_id) references nt1(id) +); +create temp table nt3 ( + id int primary key, + nt2_id int, + c1 boolean, + foreign key (nt2_id) references nt2(id) +); + +insert into nt1 values (1,true,true); +insert into nt1 values (2,true,false); +insert into nt1 values (3,false,false); +insert into nt2 values (1,1,true,true); +insert into nt2 values (2,2,true,false); +insert into nt2 values (3,3,false,false); +insert into nt3 values (1,1,true); +insert into nt3 values (2,2,false); +insert into nt3 values (3,3,true); + +explain (costs off) +select nt3.id +from nt3 as nt3 + left join + (select nt2.*, (nt2.b1 and ss1.a3) AS b3 + from nt2 as nt2 + left join + (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1 + on ss1.id = nt2.nt1_id + ) as ss2 + on ss2.id = nt3.nt2_id +where nt3.id = 1 and ss2.b3; + +select nt3.id +from nt3 as nt3 + left join + (select nt2.*, (nt2.b1 and ss1.a3) AS b3 + from nt2 as nt2 + left join + (select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1 + on ss1.id = nt2.nt1_id + ) as ss2 + on ss2.id = nt3.nt2_id +where nt3.id = 1 and ss2.b3; + -- -- test case where a PlaceHolderVar is propagated into a subquery --