Skip to content

Commit eb22950

Browse files
tglsfdctglsfdc
committed
Fix PlaceHolderVar mechanism's interaction with outer joins.
The point of a PlaceHolderVar is to allow a non-strict expression to be evaluated below an outer join, after which its value bubbles up like a Var and can be forced to NULL when the outer join's semantics require that. However, there was a serious design oversight in that, namely that we didn't ensure that there was actually a correct place in the plan tree to evaluate the placeholder :-(. It may be necessary to delay evaluation of an outer join to ensure that a placeholder that should be evaluated below the join can be evaluated there. Per recent bug report from Kirill Simonov. Back-patch to 8.4 where the PlaceHolderVar mechanism was introduced.
1 parent 9c5f4f6 commit eb22950

File tree

11 files changed

+374
-36
lines changed

11 files changed

+374
-36
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,6 +1806,7 @@ _copyPlaceHolderInfo(PlaceHolderInfo *from)
18061806
COPY_NODE_FIELD(ph_var);
18071807
COPY_BITMAPSET_FIELD(ph_eval_at);
18081808
COPY_BITMAPSET_FIELD(ph_needed);
1809+
COPY_BITMAPSET_FIELD(ph_may_need);
18091810
COPY_SCALAR_FIELD(ph_width);
18101811

18111812
return newnode;

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,7 @@ _equalPlaceHolderInfo(PlaceHolderInfo *a, PlaceHolderInfo *b)
838838
COMPARE_NODE_FIELD(ph_var);
839839
COMPARE_BITMAPSET_FIELD(ph_eval_at);
840840
COMPARE_BITMAPSET_FIELD(ph_needed);
841+
COMPARE_BITMAPSET_FIELD(ph_may_need);
841842
COMPARE_SCALAR_FIELD(ph_width);
842843

843844
return true;

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,6 +1768,7 @@ _outPlaceHolderInfo(StringInfo str, PlaceHolderInfo *node)
17681768
WRITE_NODE_FIELD(ph_var);
17691769
WRITE_BITMAPSET_FIELD(ph_eval_at);
17701770
WRITE_BITMAPSET_FIELD(ph_needed);
1771+
WRITE_BITMAPSET_FIELD(ph_may_need);
17711772
WRITE_INT_FIELD(ph_width);
17721773
}
17731774

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,8 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
396396
phinfo->ph_eval_at = bms_add_member(phinfo->ph_eval_at, relid);
397397

398398
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
399+
/* ph_may_need probably isn't used after this, but fix it anyway */
400+
phinfo->ph_may_need = bms_del_member(phinfo->ph_may_need, relid);
399401
}
400402

401403
/*

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,13 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
187187

188188
phinfo->ph_needed = bms_add_members(phinfo->ph_needed,
189189
where_needed);
190+
/*
191+
* Update ph_may_need too. This is currently only necessary
192+
* when being called from build_base_rel_tlists, but we may as
193+
* well do it always.
194+
*/
195+
phinfo->ph_may_need = bms_add_members(phinfo->ph_may_need,
196+
where_needed);
190197
}
191198
else
192199
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(node));
@@ -465,7 +472,11 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
465472

466473
/* Now we can add the SpecialJoinInfo to join_info_list */
467474
if (sjinfo)
475+
{
468476
root->join_info_list = lappend(root->join_info_list, sjinfo);
477+
/* Each time we do that, recheck placeholder eval levels */
478+
update_placeholder_eval_levels(root, sjinfo);
479+
}
469480

470481
/*
471482
* Finally, compute the output joinlist. We fold subproblems together
@@ -687,6 +698,32 @@ make_outerjoininfo(PlannerInfo *root,
687698
}
688699
}
689700

701+
/*
702+
* Examine PlaceHolderVars. If a PHV is supposed to be evaluated within
703+
* this join's nullable side, and it may get used above this join, then
704+
* ensure that min_righthand contains the full eval_at set of the PHV.
705+
* This ensures that the PHV actually can be evaluated within the RHS.
706+
* Note that this works only because we should already have determined
707+
* the final eval_at level for any PHV syntactically within this join.
708+
*/
709+
foreach(l, root->placeholder_list)
710+
{
711+
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
712+
Relids ph_syn_level = phinfo->ph_var->phrels;
713+
714+
/* Ignore placeholder if it didn't syntactically come from RHS */
715+
if (!bms_is_subset(ph_syn_level, right_rels))
716+
continue;
717+
718+
/* We can also ignore it if it's certainly not used above this join */
719+
/* XXX this test is probably overly conservative */
720+
if (bms_is_subset(phinfo->ph_may_need, min_righthand))
721+
continue;
722+
723+
/* Else, prevent join from being formed before we eval the PHV */
724+
min_righthand = bms_add_members(min_righthand, phinfo->ph_eval_at);
725+
}
726+
690727
/*
691728
* If we found nothing to put in min_lefthand, punt and make it the full
692729
* LHS, to avoid having an empty min_lefthand which will confuse later

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,19 @@ query_planner(PlannerInfo *root, List *tlist,
180180
add_base_rels_to_query(root, (Node *) parse->jointree);
181181

182182
/*
183-
* Examine the targetlist and qualifications, adding entries to baserel
184-
* targetlists for all referenced Vars. Restrict and join clauses are
185-
* added to appropriate lists belonging to the mentioned relations. We
186-
* also build EquivalenceClasses for provably equivalent expressions, and
187-
* form a target joinlist for make_one_rel() to work from.
183+
* Examine the targetlist and join tree, adding entries to baserel
184+
* targetlists for all referenced Vars, and generating PlaceHolderInfo
185+
* entries for all referenced PlaceHolderVars. Restrict and join clauses
186+
* are added to appropriate lists belonging to the mentioned relations.
187+
* We also build EquivalenceClasses for provably equivalent expressions.
188+
* The SpecialJoinInfo list is also built to hold information about join
189+
* order restrictions. Finally, we form a target joinlist for
190+
* make_one_rel() to work from.
188191
*/
189192
build_base_rel_tlists(root, tlist);
190193

194+
find_placeholders_in_jointree(root);
195+
191196
joinlist = deconstruct_jointree(root);
192197

193198
/*
@@ -218,10 +223,12 @@ query_planner(PlannerInfo *root, List *tlist,
218223

219224
/*
220225
* Examine any "placeholder" expressions generated during subquery pullup.
221-
* Make sure that we know what level to evaluate them at, and that the
222-
* Vars they need are marked as needed.
226+
* Make sure that the Vars they need are marked as needed at the relevant
227+
* join level. This must be done before join removal because it might
228+
* cause Vars or placeholders to be needed above a join when they weren't
229+
* so marked before.
223230
*/
224-
fix_placeholder_eval_levels(root);
231+
fix_placeholder_input_needed_levels(root);
225232

226233
/*
227234
* Remove any useless outer joins. Ideally this would be done during

0 commit comments

Comments
 (0)