Skip to content

Commit 187ae17

Browse files
tglsfdctglsfdc
committed
Fix planner bug with nested PlaceHolderVars in 9.2 (only).
Commit 9e7e29c fixed some problems with LATERAL references in PlaceHolderVars, one of which was that "createplan.c wasn't handling nested PlaceHolderVars properly". I failed to see that this problem might occur in older versions as well; but it can, as demonstrated in bug #10587 from Geoff Speicher. In this case the nesting occurs due to push-down of PlaceHolderVar expressions into a parameterized path. So, back-patch the relevant changes from 9e7e29c into 9.2 where parameterized paths were introduced. (Perhaps I'm still being too myopic, but I'm hesitant to change older branches without some evidence that the case can occur there.)
1 parent 93328b2 commit 187ae17

File tree

4 files changed

+172
-15
lines changed

4 files changed

+172
-15
lines changed

‎src/backend/nodes/equalfuncs.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -759,15 +759,19 @@ _equalPlaceHolderVar(const PlaceHolderVar *a, const PlaceHolderVar *b)
759759
/*
760760
* We intentionally do not compare phexpr. Two PlaceHolderVars with the
761761
* same ID and levelsup should be considered equal even if the contained
762-
* expressions have managed to mutate to different states. One way in
763-
* which that can happen is that initplan sublinks would get replaced by
764-
* differently-numbered Params when sublink folding is done. (The end
765-
* result of such a situation would be some unreferenced initplans, which
766-
* is annoying but not really a problem.)
762+
* expressions have managed to mutate to different states. This will
763+
* happen during final plan construction when there are nested PHVs, since
764+
* the inner PHV will get replaced by a Param in some copies of the outer
765+
* PHV. Another way in which it can happen is that initplan sublinks
766+
* could get replaced by differently-numbered Params when sublink folding
767+
* is done. (The end result of such a situation would be some
768+
* unreferenced initplans, which is annoying but not really a problem.) On
769+
* the same reasoning, there is no need to examine phrels.
767770
*
768771
* COMPARE_NODE_FIELD(phexpr);
772+
*
773+
* COMPARE_BITMAPSET_FIELD(phrels);
769774
*/
770-
COMPARE_BITMAPSET_FIELD(phrels);
771775
COMPARE_SCALAR_FIELD(phid);
772776
COMPARE_SCALAR_FIELD(phlevelsup);
773777

‎src/backend/optimizer/plan/createplan.c

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,16 +2532,37 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
25322532
Assert(phv->phlevelsup == 0);
25332533

25342534
/*
2535-
* If not to be replaced, just return the PlaceHolderVar unmodified.
2536-
* We use bms_overlap as a cheap/quick test to see if the PHV might be
2537-
* evaluated in the outer rels, and then grab its PlaceHolderInfo to
2538-
* tell for sure.
2535+
* Check whether we need to replace the PHV. We use bms_overlap as a
2536+
* cheap/quick test to see if the PHV might be evaluated in the outer
2537+
* rels, and then grab its PlaceHolderInfo to tell for sure.
25392538
*/
2540-
if (!bms_overlap(phv->phrels, root->curOuterRels))
2541-
return node;
2542-
if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at,
2543-
root->curOuterRels))
2544-
return node;
2539+
if (!bms_overlap(phv->phrels, root->curOuterRels) ||
2540+
!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at,
2541+
root->curOuterRels))
2542+
{
2543+
/*
2544+
* We can't replace the whole PHV, but we might still need to
2545+
* replace Vars or PHVs within its expression, in case it ends up
2546+
* actually getting evaluated here. (It might get evaluated in
2547+
* this plan node, or some child node; in the latter case we don't
2548+
* really need to process the expression here, but we haven't got
2549+
* enough info to tell if that's the case.) Flat-copy the PHV
2550+
* node and then recurse on its expression.
2551+
*
2552+
* Note that after doing this, we might have different
2553+
* representations of the contents of the same PHV in different
2554+
* parts of the plan tree. This is OK because equal() will just
2555+
* match on phid/phlevelsup, so setrefs.c will still recognize an
2556+
* upper-level reference to a lower-level copy of the same PHV.
2557+
*/
2558+
PlaceHolderVar *newphv = makeNode(PlaceHolderVar);
2559+
2560+
memcpy(newphv, phv, sizeof(PlaceHolderVar));
2561+
newphv->phexpr = (Expr *)
2562+
replace_nestloop_params_mutator((Node *) phv->phexpr,
2563+
root);
2564+
return (Node *) newphv;
2565+
}
25452566
/* Create a Param representing the PlaceHolderVar */
25462567
param = assign_nestloop_param_placeholdervar(root, phv);
25472568
/* Is this param already listed in root->curOuterParams? */

‎src/test/regress/expected/join.out

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2570,6 +2570,80 @@ SELECT qq, unique1
25702570
456 | 7318
25712571
(3 rows)
25722572

2573+
--
2574+
-- nested nestloops can require nested PlaceHolderVars
2575+
--
2576+
create temp table nt1 (
2577+
id int primary key,
2578+
a1 boolean,
2579+
a2 boolean
2580+
);
2581+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "nt1_pkey" for table "nt1"
2582+
create temp table nt2 (
2583+
id int primary key,
2584+
nt1_id int,
2585+
b1 boolean,
2586+
b2 boolean,
2587+
foreign key (nt1_id) references nt1(id)
2588+
);
2589+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "nt2_pkey" for table "nt2"
2590+
create temp table nt3 (
2591+
id int primary key,
2592+
nt2_id int,
2593+
c1 boolean,
2594+
foreign key (nt2_id) references nt2(id)
2595+
);
2596+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "nt3_pkey" for table "nt3"
2597+
insert into nt1 values (1,true,true);
2598+
insert into nt1 values (2,true,false);
2599+
insert into nt1 values (3,false,false);
2600+
insert into nt2 values (1,1,true,true);
2601+
insert into nt2 values (2,2,true,false);
2602+
insert into nt2 values (3,3,false,false);
2603+
insert into nt3 values (1,1,true);
2604+
insert into nt3 values (2,2,false);
2605+
insert into nt3 values (3,3,true);
2606+
explain (costs off)
2607+
select nt3.id
2608+
from nt3 as nt3
2609+
left join
2610+
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
2611+
from nt2 as nt2
2612+
left join
2613+
(select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
2614+
on ss1.id = nt2.nt1_id
2615+
) as ss2
2616+
on ss2.id = nt3.nt2_id
2617+
where nt3.id = 1 and ss2.b3;
2618+
QUERY PLAN
2619+
-----------------------------------------------
2620+
Nested Loop
2621+
-> Nested Loop
2622+
-> Index Scan using nt3_pkey on nt3
2623+
Index Cond: (id = 1)
2624+
-> Index Scan using nt2_pkey on nt2
2625+
Index Cond: (id = nt3.nt2_id)
2626+
-> Index Only Scan using nt1_pkey on nt1
2627+
Index Cond: (id = nt2.nt1_id)
2628+
Filter: (nt2.b1 AND (id IS NOT NULL))
2629+
(9 rows)
2630+
2631+
select nt3.id
2632+
from nt3 as nt3
2633+
left join
2634+
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
2635+
from nt2 as nt2
2636+
left join
2637+
(select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
2638+
on ss1.id = nt2.nt1_id
2639+
) as ss2
2640+
on ss2.id = nt3.nt2_id
2641+
where nt3.id = 1 and ss2.b3;
2642+
id
2643+
----
2644+
1
2645+
(1 row)
2646+
25732647
--
25742648
-- test case where a PlaceHolderVar is propagated into a subquery
25752649
--

‎src/test/regress/sql/join.sql

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,64 @@ SELECT qq, unique1
660660
USING (qq)
661661
INNER JOIN tenk1 c ON qq = unique2;
662662

663+
--
664+
-- nested nestloops can require nested PlaceHolderVars
665+
--
666+
667+
create temp table nt1 (
668+
id int primary key,
669+
a1 boolean,
670+
a2 boolean
671+
);
672+
create temp table nt2 (
673+
id int primary key,
674+
nt1_id int,
675+
b1 boolean,
676+
b2 boolean,
677+
foreign key (nt1_id) references nt1(id)
678+
);
679+
create temp table nt3 (
680+
id int primary key,
681+
nt2_id int,
682+
c1 boolean,
683+
foreign key (nt2_id) references nt2(id)
684+
);
685+
686+
insert into nt1 values (1,true,true);
687+
insert into nt1 values (2,true,false);
688+
insert into nt1 values (3,false,false);
689+
insert into nt2 values (1,1,true,true);
690+
insert into nt2 values (2,2,true,false);
691+
insert into nt2 values (3,3,false,false);
692+
insert into nt3 values (1,1,true);
693+
insert into nt3 values (2,2,false);
694+
insert into nt3 values (3,3,true);
695+
696+
explain (costs off)
697+
select nt3.id
698+
from nt3 as nt3
699+
left join
700+
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
701+
from nt2 as nt2
702+
left join
703+
(select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
704+
on ss1.id = nt2.nt1_id
705+
) as ss2
706+
on ss2.id = nt3.nt2_id
707+
where nt3.id = 1 and ss2.b3;
708+
709+
select nt3.id
710+
from nt3 as nt3
711+
left join
712+
(select nt2.*, (nt2.b1 and ss1.a3) AS b3
713+
from nt2 as nt2
714+
left join
715+
(select nt1.*, (nt1.id is not null) as a3 from nt1) as ss1
716+
on ss1.id = nt2.nt1_id
717+
) as ss2
718+
on ss2.id = nt3.nt2_id
719+
where nt3.id = 1 and ss2.b3;
720+
663721
--
664722
-- test case where a PlaceHolderVar is propagated into a subquery
665723
--

0 commit comments

Comments
 (0)