From 1f0363001166ef6a43619846e44cfb9dbe7335ed Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 20 Apr 2012 20:10:46 -0400 Subject: [PATCH] Adjust join_search_one_level's handling of clauseless joins. For an initial relation that lacks any join clauses (that is, it has to be cartesian-product-joined to the rest of the query), we considered only cartesian joins with initial rels appearing later in the initial-relations list. This creates an undesirable dependency on FROM-list order. We would never fail to find a plan, but perhaps we might not find the best available plan. Noted while discussing the logic with Amit Kapila. Improve the comments a bit in this area, too. Arguably this is a bug fix, but given the lack of complaints from the field I'll refrain from back-patching. --- src/backend/optimizer/path/joinrels.c | 46 ++++++++++++++++----------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index 82252753fb..24d4651507 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -65,33 +65,34 @@ join_search_one_level(PlannerInfo *root, int level) * We prefer to join using join clauses, but if we find a rel of level-1 * members that has no join clauses, we will generate Cartesian-product * joins against all initial rels not already contained in it. - * - * In the first pass (level == 2), we try to join each initial rel to each - * initial rel that appears later in joinrels[1]. (The mirror-image joins - * are handled automatically by make_join_rel.) In later passes, we try - * to join rels of size level-1 from joinrels[level-1] to each initial rel - * in joinrels[1]. */ foreach(r, joinrels[level - 1]) { RelOptInfo *old_rel = (RelOptInfo *) lfirst(r); - ListCell *other_rels; - - if (level == 2) - other_rels = lnext(r); /* only consider remaining initial - * rels */ - else - other_rels = list_head(joinrels[1]); /* consider all initial - * rels */ if (old_rel->joininfo != NIL || old_rel->has_eclass_joins || has_join_restriction(root, old_rel)) { /* - * There are relevant join clauses or join order restrictions, - * so consider joins between this rel and (only) those rels it is - * linked to by a clause or restriction. + * There are join clauses or join order restrictions relevant to + * this rel, so consider joins between this rel and (only) those + * initial rels it is linked to by a clause or restriction. + * + * At level 2 this condition is symmetric, so there is no need to + * look at initial rels before this one in the list; we already + * considered such joins when we were at the earlier rel. (The + * mirror-image joins are handled automatically by make_join_rel.) + * In later passes (level > 2), we join rels of the previous level + * to each initial rel they don't already include but have a join + * clause or restriction with. */ + ListCell *other_rels; + + if (level == 2) /* consider remaining initial rels */ + other_rels = lnext(r); + else /* consider all initial rels */ + other_rels = list_head(joinrels[1]); + make_rels_by_clause_joins(root, old_rel, other_rels); @@ -102,10 +103,17 @@ join_search_one_level(PlannerInfo *root, int level) * Oops, we have a relation that is not joined to any other * relation, either directly or by join-order restrictions. * Cartesian product time. + * + * We consider a cartesian product with each not-already-included + * initial rel, whether it has other join clauses or not. At + * level 2, if there are two or more clauseless initial rels, we + * will redundantly consider joining them in both directions; but + * such cases aren't common enough to justify adding complexity to + * avoid the duplicated effort. */ make_rels_by_clauseless_joins(root, old_rel, - other_rels); + list_head(joinrels[1])); } } @@ -135,7 +143,7 @@ join_search_one_level(PlannerInfo *root, int level) ListCell *r2; /* - * We can ignore clauseless joins here, *except* when they + * We can ignore relations without join clauses here, unless they * participate in join-order restrictions --- then we might have * to force a bushy join plan. */