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.

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 82252753fb0d331875c1607a7f7169601b6183d6..24d46515070e8602d31db2fc9dc264c9ec6543b5 100644 (file)
--- 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.
                         */