Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit acfcd45

Browse files
committed
Still more fixes for planner's handling of LATERAL references.
More fuzz testing by Andreas Seltenreich exposed that the planner did not cope well with chains of lateral references. If relation X references Y laterally, and Y references Z laterally, then we will have to scan X on the inside of a nestloop with Z, so for all intents and purposes X is laterally dependent on Z too. The planner did not understand this and would generate intermediate joins that could not be used. While that was usually harmless except for wasting some planning cycles, under the right circumstances it would lead to "failed to build any N-way joins" or "could not devise a query plan" planner failures. To fix that, convert the existing per-relation lateral_relids and lateral_referencers relid sets into their transitive closures; that is, they now show all relations on which a rel is directly or indirectly laterally dependent. This not only fixes the chained-reference problem but allows some of the relevant tests to be made substantially simpler and faster, since they can be reduced to simple bitmap manipulations instead of searches of the LateralJoinInfo list. Also, when a PlaceHolderVar that is due to be evaluated at a join contains lateral references, we should treat those references as indirect lateral dependencies of each of the join's base relations. This prevents us from trying to join any individual base relations to the lateral reference source before the join is formed, which again cannot work. Andreas' testing also exposed another oversight in the "dangerous PlaceHolderVar" test added in commit 85e5e22. Simply rejecting unsafe join paths in joinpath.c is insufficient, because in some cases we will end up rejecting *all* possible paths for a particular join, again leading to "could not devise a query plan" failures. The restriction has to be known also to join_is_legal and its cohort functions, so that they will not select a join for which that will happen. I chose to move the supporting logic into joinrels.c where the latter functions are. Back-patch to 9.3 where LATERAL support was introduced.
1 parent 69e7235 commit acfcd45

File tree

9 files changed

+528
-187
lines changed

9 files changed

+528
-187
lines changed

src/backend/optimizer/path/joinpath.c

+6-73
Original file line numberDiff line numberDiff line change
@@ -254,54 +254,6 @@ allow_star_schema_join(PlannerInfo *root,
254254
bms_nonempty_difference(innerparams, outerrelids));
255255
}
256256

257-
/*
258-
* There's a pitfall for creating parameterized nestloops: suppose the inner
259-
* rel (call it A) has a parameter that is a PlaceHolderVar, and that PHV's
260-
* minimum eval_at set includes the outer rel (B) and some third rel (C).
261-
* We might think we could create a B/A nestloop join that's parameterized by
262-
* C. But we would end up with a plan in which the PHV's expression has to be
263-
* evaluated as a nestloop parameter at the B/A join; and the executor is only
264-
* set up to handle simple Vars as NestLoopParams. Rather than add complexity
265-
* and overhead to the executor for such corner cases, it seems better to
266-
* forbid the join. (Note that existence of such a PHV probably means there
267-
* is a join order constraint that will cause us to consider joining B and C
268-
* directly; so we can still make use of A's parameterized path with B+C.)
269-
* So we check whether any PHVs used in the query could pose such a hazard.
270-
* We don't have any simple way of checking whether a risky PHV would actually
271-
* be used in the inner plan, and the case is so unusual that it doesn't seem
272-
* worth working very hard on it.
273-
*
274-
* This case can occur whether or not the join's remaining parameterization
275-
* overlaps param_source_rels, so we have to check for it separately from
276-
* allow_star_schema_join, even though it looks much like a star-schema case.
277-
*/
278-
static inline bool
279-
check_hazardous_phv(PlannerInfo *root,
280-
Path *outer_path,
281-
Path *inner_path)
282-
{
283-
Relids innerparams = PATH_REQ_OUTER(inner_path);
284-
Relids outerrelids = outer_path->parent->relids;
285-
ListCell *lc;
286-
287-
foreach(lc, root->placeholder_list)
288-
{
289-
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
290-
291-
if (!bms_is_subset(phinfo->ph_eval_at, innerparams))
292-
continue; /* ignore, could not be a nestloop param */
293-
if (!bms_overlap(phinfo->ph_eval_at, outerrelids))
294-
continue; /* ignore, not relevant to this join */
295-
if (bms_is_subset(phinfo->ph_eval_at, outerrelids))
296-
continue; /* safe, it can be eval'd within outerrel */
297-
/* Otherwise, it's potentially unsafe, so reject the join */
298-
return false;
299-
}
300-
301-
/* OK to perform the join */
302-
return true;
303-
}
304-
305257
/*
306258
* try_nestloop_path
307259
* Consider a nestloop join path; if it appears useful, push it into
@@ -322,31 +274,24 @@ try_nestloop_path(PlannerInfo *root,
322274
/*
323275
* Check to see if proposed path is still parameterized, and reject if the
324276
* parameterization wouldn't be sensible --- unless allow_star_schema_join
325-
* says to allow it anyway. Also, we must reject if check_hazardous_phv
326-
* doesn't like the look of it.
277+
* says to allow it anyway. Also, we must reject if have_dangerous_phv
278+
* doesn't like the look of it, which could only happen if the nestloop is
279+
* still parameterized.
327280
*/
328281
required_outer = calc_nestloop_required_outer(outer_path,
329282
inner_path);
330283
if (required_outer &&
331284
((!bms_overlap(required_outer, extra->param_source_rels) &&
332285
!allow_star_schema_join(root, outer_path, inner_path)) ||
333-
!check_hazardous_phv(root, outer_path, inner_path)))
286+
have_dangerous_phv(root,
287+
outer_path->parent->relids,
288+
PATH_REQ_OUTER(inner_path))))
334289
{
335290
/* Waste no memory when we reject a path here */
336291
bms_free(required_outer);
337292
return;
338293
}
339294

340-
/*
341-
* Independently of that, add parameterization needed for any
342-
* PlaceHolderVars that need to be computed at the join. We can handle
343-
* that just by adding joinrel->lateral_relids; that might include some
344-
* rels that are already in required_outer, but no harm done. (Note that
345-
* lateral_relids is exactly NULL if empty, so this will not break the
346-
* property that required_outer is too.)
347-
*/
348-
required_outer = bms_add_members(required_outer, joinrel->lateral_relids);
349-
350295
/*
351296
* Do a precheck to quickly eliminate obviously-inferior paths. We
352297
* calculate a cheap lower bound on the path's cost and then use
@@ -418,12 +363,6 @@ try_mergejoin_path(PlannerInfo *root,
418363
return;
419364
}
420365

421-
/*
422-
* Independently of that, add parameterization needed for any
423-
* PlaceHolderVars that need to be computed at the join.
424-
*/
425-
required_outer = bms_add_members(required_outer, joinrel->lateral_relids);
426-
427366
/*
428367
* If the given paths are already well enough ordered, we can skip doing
429368
* an explicit sort.
@@ -500,12 +439,6 @@ try_hashjoin_path(PlannerInfo *root,
500439
return;
501440
}
502441

503-
/*
504-
* Independently of that, add parameterization needed for any
505-
* PlaceHolderVars that need to be computed at the join.
506-
*/
507-
required_outer = bms_add_members(required_outer, joinrel->lateral_relids);
508-
509442
/*
510443
* See comments in try_nestloop_path(). Also note that hashjoin paths
511444
* never have any output pathkeys, per comments in create_hashjoin_path.

0 commit comments

Comments
 (0)