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 00e8d6f13fe163a93d971574a487708f7416e2f8..ef0b25bf5e22886adccba280bd05190f881367b4 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.167 2008/01/01 19:45:50 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.168 2008/01/11 04:02:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -674,7 +674,12 @@ make_rel_from_joinlist(PlannerInfo *root, List *joinlist)
        /*
         * Consider the different orders in which we could join the rels,
         * using a plugin, GEQO, or the regular join search code.
+        *
+        * 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 (join_search_hook)
            return (*join_search_hook) (root, levels_needed, initial_rels);
        else if (enable_geqo && levels_needed >= geqo_threshold)

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 1287037c0ba60fdc17b498b54789c67aa3c1d8a7..8df5c21f1fd3d246f70e0e2c7848e9dd4cf726f7 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.90 2008/01/01 19:45:50 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.91 2008/01/11 04:02:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -838,33 +838,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 9edd552138edabc8ae85f02d892a6483533874f1..3776cf9b3478e1336024392c78be40acfbc001f2 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.105 2008/01/01 19:45:50 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.106 2008/01/11 04:02:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -142,6 +142,7 @@ query_planner(PlannerInfo *root, List *tlist,
    root->right_join_clauses = NIL;
    root->full_join_clauses = NIL;
    root->oj_info_list = NIL;
+   root->initial_rels = NIL;
 
    /*
     * Make a flattened version of the rangetable for faster access (this is

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index f01f1e9e51f5d9132d3ccbd205f1f159862e6c6a..b106c590e3e1b0e840ba0849450de4558e8e23c1 100644 (file)
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.153 2008/01/09 20:42:28 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.154 2008/01/11 04:02:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -165,6 +165,8 @@ typedef struct PlannerInfo
    List       *group_pathkeys; /* groupClause pathkeys, if any */
    List       *sort_pathkeys;  /* sortClause pathkeys, if any */
 
+   List       *initial_rels;   /* RelOptInfos we are now trying to join */
+
    MemoryContext planner_cxt;  /* context holding PlannerInfo */
 
    double      total_table_pages;      /* # of pages in all tables of query */