Fix a conceptual error in my patch of 2007-10-26 that avoided considering

clauseless joins of relations that have unexploited join clauses.  Rather
than looking at every other base relation in the query, the correct thing is
to examine the other relations in the "initial_rels" list of the current
make_rel_from_joinlist() invocation, because those are what we actually have
the ability to join against.  This might be a subset of the whole query in
cases where join_collapse_limit or from_collapse_limit or full joins have
prevented merging the whole query into a single join problem.  This is a bit
untidy because we have to pass those rels down through a new PlannerInfo
field, but it's necessary.  Per bug #3865 from Oleg Kharin.

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 1e195398f3fead23933659d9d1067dc9cb8ee1c2..bd8c7816c40ec3ef0aed9b3af1ae539c07be0c44 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.154.2.1 2007/01/28 18:50:48 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.154.2.2 2008/01/11 04:02:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -628,7 +628,12 @@ make_rel_from_joinlist(PlannerInfo *root, List *joinlist)
                /*
                 * Consider the different orders in which we could join the rels,
                 * using either GEQO or regular optimizer.
+                *
+                * We put the initial_rels list into a PlannerInfo field because
+                * has_legal_joinclause() needs to look at it (ugly :-().
                 */
+               root->initial_rels = initial_rels;
+
                if (enable_geqo && levels_needed >= geqo_threshold)
                        return geqo(root, levels_needed, initial_rels);
                else

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index fcb36c9b22ec8d7883417a164a4b66350f71d382..12d7a2295c7b868bfb698ffe3f1d0fe44fdb8a9c 100644 (file)
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.81.2.4 2007/10/26 18:10:58 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.81.2.5 2008/01/11 04:02:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -837,33 +837,27 @@ has_join_restriction(PlannerInfo *root, RelOptInfo *rel)
  *             Detect whether the specified relation can legally be joined
  *             to any other rels using join clauses.
  *
- * We consider only joins to single other relations.  This is sufficient
- * to get a "true" result in most real queries, and an occasional erroneous
- * "false" will only cost a bit more planning time.  The reason for this
- * limitation is that considering joins to other joins would require proving
- * that the other join rel can legally be formed, which seems like too much
- * trouble for something that's only a heuristic to save planning time.
+ * We consider only joins to single other relations in the current
+ * initial_rels list.  This is sufficient to get a "true" result in most real
+ * queries, and an occasional erroneous "false" will only cost a bit more
+ * planning time.  The reason for this limitation is that considering joins to
+ * other joins would require proving that the other join rel can legally be
+ * formed, which seems like too much trouble for something that's only a
+ * heuristic to save planning time.  (Note: we must look at initial_rels
+ * and not all of the query, since when we are planning a sub-joinlist we
+ * may be forced to make clauseless joins within initial_rels even though
+ * there are join clauses linking to other parts of the query.)
  */
 static bool
 has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel)
 {
-       Index           rti;
+       ListCell   *lc;
 
-       for (rti = 1; rti < root->simple_rel_array_size; rti++)
+       foreach(lc, root->initial_rels)
        {
-               RelOptInfo *rel2 = root->simple_rel_array[rti];
+               RelOptInfo *rel2 = (RelOptInfo *) lfirst(lc);
 
-               /* there may be empty slots corresponding to non-baserel RTEs */
-               if (rel2 == NULL)
-                       continue;
-
-               Assert(rel2->relid == rti);             /* sanity check on array */
-
-               /* ignore RTEs that are "other rels" */
-               if (rel2->reloptkind != RELOPT_BASEREL)
-                       continue;
-
-               /* ignore RTEs that are already in "rel" */
+               /* ignore rels that are already in "rel" */
                if (bms_overlap(rel->relids, rel2->relids))
                        continue;
 

diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index 1dc9e97fdb690a2d6b4147985548ced3fc600a5d..de43c96a5799d19f771aca2dbfa901ac648a7189 100644 (file)
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.97.2.1 2007/10/04 20:44:55 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.97.2.2 2008/01/11 04:02:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -122,6 +122,7 @@ query_planner(PlannerInfo *root, List *tlist, double tuple_fraction,
        root->right_join_clauses = NIL;
        root->full_join_clauses = NIL;
        root->oj_info_list = NIL;
+       root->initial_rels = NIL;
 
        /*
         * Construct RelOptInfo nodes for all base relations in query, and

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 34727773e0667fc63866622ce22333b00c179aaa..865c7caff6650c9e8edab81bc0f98621fb9c2f3e 100644 (file)
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.128.2.4 2007/08/31 01:44:14 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.128.2.5 2008/01/11 04:02:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -118,6 +118,9 @@ typedef struct PlannerInfo
        bool            hasHavingQual;  /* true if havingQual was non-null */
        bool            hasPseudoConstantQuals; /* true if any RestrictInfo has
                                                                                 * pseudoconstant = true */
+
+       /* At the end to avoid breaking existing 8.2 add-ons */
+       List       *initial_rels;       /* RelOptInfos we are now trying to join */
 } PlannerInfo;