database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-07 00:36:50 +03:00
Code Activity

Fix miscomputation of direct_lateral_relids for join relations.

Browse Source 
If a PlaceHolderVar is to be evaluated at a join relation, but
its value is only needed there and not at higher levels, we neglected
to update the joinrel's direct_lateral_relids to include the PHV's
source rel.  This causes problems because join_is_legal() then won't
allow joining the joinrel to the PHV's source rel at all, leading
to "failed to build any N-way joins" planner failures.

Per report from Andreas Seltenreich.  Back-patch to 9.5
where the problem originated.

Discussion: https://postgr.es/m/87blfgqa4t.fsf@aurora.ydns.eu
This commit is contained in:
Tom Lane
2020-11-30 12:22:43 -05:00
parent 873ea9ee69
commit b1738ff6ab
3 changed files with 118 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
src/backend/optimizer/util/placeholder.c
View File

@ -404,8 +404,10 @@ add_placeholders_to_base_rels(PlannerInfo *root)
* and if they contain lateral references, add those references to the
* joinrel's direct_lateral_relids.
*
* A join rel should emit a PlaceHolderVar if (a) the PHV is needed above
* this join level and (b) the PHV can be computed at or below this level.
* A join rel should emit a PlaceHolderVar if (a) the PHV can be computed
* at or below this join level and (b) the PHV is needed above this level.
* However, condition (a) is sufficient to add to direct_lateral_relids,
* as explained below.
*/
void
add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel,
@ -418,11 +420,11 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel,
{
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
/* Is it still needed above this joinrel? */
if (bms_nonempty_difference(phinfo->ph_needed, relids))
/* Is it computable here? */
if (bms_is_subset(phinfo->ph_eval_at, relids))
{
/* Is it computable here? */
if (bms_is_subset(phinfo->ph_eval_at, relids))
/* Is it still needed above this joinrel? */
if (bms_nonempty_difference(phinfo->ph_needed, relids))
{
/* Yup, add it to the output */
joinrel->reltarget->exprs = lappend(joinrel->reltarget->exprs,
@ -450,12 +452,26 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel,
joinrel->reltarget->cost.startup += cost.startup;
joinrel->reltarget->cost.per_tuple += cost.per_tuple;
}
/* Adjust joinrel's direct_lateral_relids as needed */
joinrel->direct_lateral_relids =
bms_add_members(joinrel->direct_lateral_relids,
phinfo->ph_lateral);
}
/*
* Also adjust joinrel's direct_lateral_relids to include the
* PHV's source rel(s). We must do this even if we're not
* actually going to emit the PHV, otherwise join_is_legal() will
* reject valid join orderings. (In principle maybe we could
* instead remove the joinrel's lateral_relids dependency; but
* that's complicated to get right, and cases where we're not
* going to emit the PHV are too rare to justify the work.)
*
* In principle we should only do this if the join doesn't yet
* include the PHV's source rel(s). But our caller
* build_join_rel() will clean things up by removing the join's
* own relids from its direct_lateral_relids, so we needn't
* account for that here.
*/
joinrel->direct_lateral_relids =
bms_add_members(joinrel->direct_lateral_relids,
phinfo->ph_lateral);
}
}
}