Fix an oversight in the code that makes transitive-equality deductions from

outer join clauses.  Given, say,
	... from a left join b on a.a1 = b.b1 where a.a1 = 42;
we'll deduce a clause b.b1 = 42 and then mark the original join clause
redundant (we can't remove it completely for reasons I don't feel like
squeezing into this log entry).  However the original implementation of
that wasn't bulletproof, because clause_selectivity() wouldn't honor
this_selec if given nonzero varRelid --- which in practice meant that
it worked as desired *except* when considering index scan quals.  Which
resulted in bogus underestimation of the size of the indexscan result for
an inner indexscan in an outer join, and consequently a possibly bad
choice of indexscan vs. bitmap scan.  Fix by introducing an explicit test
into clause_selectivity().  Also, to make sure we don't trigger that test
in corner cases, change the convention to be that this_selec > 1, not
this_selec = 1, means it's been marked redundant.  Per trouble report from
Scara Maccai.

Back-patch to 8.2, where the problem was introduced.

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ac6787688d777302127724e51ea04681d41add8f..b1080433e798d4db3e67f7bcfd946dfbeac1ec23 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.285.2.4 2007/08/31 01:44:14 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.285.2.5 2008/12/01 21:06:30 tgl Exp $
  *
  * NOTES
  *       Every node type that can appear in stored rules' parsetrees *must*
@@ -1266,6 +1266,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
        WRITE_BITMAPSET_FIELD(left_relids);
        WRITE_BITMAPSET_FIELD(right_relids);
        WRITE_NODE_FIELD(orclause);
+       WRITE_FLOAT_FIELD(this_selec, "%.4f");
        WRITE_OID_FIELD(mergejoinoperator);
        WRITE_OID_FIELD(left_sortop);
        WRITE_OID_FIELD(right_sortop);

diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c
index 463408f26fd03d301135300077bcf603ace1e231..ccab70796e6d0df50703925255e9d7ae0bc8dd7f 100644 (file)
--- a/src/backend/optimizer/path/clausesel.c
+++ b/src/backend/optimizer/path/clausesel.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/clausesel.c,v 1.82.2.1 2007/08/31 23:35:29 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/clausesel.c,v 1.82.2.2 2008/12/01 21:06:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -444,6 +444,12 @@ clause_selectivity(PlannerInfo *root,
                                return s1;
                }
 
+               /*
+                * If the clause is marked redundant, always return 1.0.
+                */
+               if (rinfo->this_selec > 1)
+                       return (Selectivity) 1.0;
+
                /*
                 * If possible, cache the result of the selectivity calculation for
                 * the clause.  We can cache if varRelid is zero or the clause

diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index abc50ef14d07bc2bad6cdf460982d8c685c89c69..6550ee4f241228ddee397ea772ac31b4e6fdf148 100644 (file)
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/pathkeys.c,v 1.79.2.1 2008/01/09 20:50:11 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/pathkeys.c,v 1.79.2.2 2008/12/01 21:06:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -428,11 +428,11 @@ sub_generate_join_implications(PlannerInfo *root,
                         * its sides to the same value.  However, that fails in some
                         * corner cases where lower outer joins could cause one of the
                         * variables to go to NULL.  (BUG in 8.2 through 8.2.6.)
-                        * So now we just leave it in place, but mark it with selectivity
-                        * 1.0 so that we don't underestimate the join size output ---
+                        * So now we just leave it in place, but mark it as redundant
+                        * so that we don't underestimate the join size output ---
                         * it's mostly redundant with the constant constraints.
                         */
-                       rinfo->this_selec = 1.0;
+                       rinfo->this_selec = 2.0;
 
                        /*
                         * And recurse to see if we can deduce anything from INNERVAR =
@@ -471,11 +471,11 @@ sub_generate_join_implications(PlannerInfo *root,
                         * its sides to the same value.  However, that fails in some
                         * corner cases where lower outer joins could cause one of the
                         * variables to go to NULL.  (BUG in 8.2 through 8.2.6.)
-                        * So now we just leave it in place, but mark it with selectivity
-                        * 1.0 so that we don't underestimate the join size output ---
+                        * So now we just leave it in place, but mark it as redundant
+                        * so that we don't underestimate the join size output ---
                         * it's mostly redundant with the constant constraints.
                         */
-                       rinfo->this_selec = 1.0;
+                       rinfo->this_selec = 2.0;
 
                        /*
                         * And recurse to see if we can deduce anything from INNERVAR =
@@ -551,15 +551,15 @@ sub_generate_join_implications(PlannerInfo *root,
                                 * of its sides to the same value.  However, that fails in
                                 * some corner cases where lower outer joins could cause one
                                 * of the variables to go to NULL.  (BUG in 8.2 through
-                                * 8.2.6.)  So now we just leave it in place, but mark it with
-                                * selectivity 1.0 so that we don't underestimate the join
+                                * 8.2.6.)  So now we just leave it in place, but mark it as
+                                * redundant so that we don't underestimate the join
                                 * size output --- it's mostly redundant with the constant
                                 * constraints.
                                 *
                                 * Ideally we'd do that for the COALESCE() = CONSTANT rinfo,
                                 * too, but we don't have easy access to that here.
                                 */
-                               rinfo->this_selec = 1.0;
+                               rinfo->this_selec = 2.0;
 
                                /*
                                 * And recurse to see if we can deduce anything from LEFTVAR =

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 865c7caff6650c9e8edab81bc0f98621fb9c2f3e..e6bae2ada0572309aa29520c60a14b366ce92242 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.5 2008/01/11 04:02:26 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.128.2.6 2008/12/01 21:06:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -777,7 +777,8 @@ typedef struct RestrictInfo
 
        /* cache space for cost and selectivity */
        QualCost        eval_cost;              /* eval cost of clause; -1 if not yet set */
-       Selectivity this_selec;         /* selectivity; -1 if not yet set */
+       Selectivity this_selec;         /* selectivity; -1 if not yet set; >1 means
+                                                                * a redundant clause */
 
        /* valid if clause is mergejoinable, else InvalidOid: */
        Oid                     mergejoinoperator;              /* copy of clause operator */