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 ddd79d42ad746e24907b6d9229d7768df95114e7..f0af743064a49c7d418f93fdf0326df38b658ed7 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1563,6 +1563,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
        WRITE_BITMAPSET_FIELD(right_relids);
        WRITE_NODE_FIELD(orclause);
        /* don't write parent_ec, leads to infinite recursion in plan tree dump */
+       WRITE_FLOAT_FIELD(this_selec, "%.4f");
        WRITE_NODE_FIELD(mergeopfamilies);
        /* don't write left_ec, leads to infinite recursion in plan tree dump */
        /* don't write right_ec, leads to infinite recursion in plan tree dump */

diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c
index 28e6fd1b7048a8b67909081e69ee40aa21cf93d1..6dca9c7596c2ca29407215919b98680b85245918 100644 (file)
--- a/src/backend/optimizer/path/clausesel.c
+++ b/src/backend/optimizer/path/clausesel.c
@@ -513,6 +513,12 @@ clause_selectivity(PlannerInfo *root,
                                return (Selectivity) 1.0;
                }
 
+               /*
+                * 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/equivclass.c b/src/backend/optimizer/path/equivclass.c
index ceec86a9c17848c9a7e407797de0beca332426d9..f85b0f1ab6e1ecec388656d12172f9958e1a4722 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -1198,7 +1198,7 @@ reconsider_outer_join_clauses(PlannerInfo *root)
                                        list_delete_cell(root->left_join_clauses, cell, prev);
                                /* we throw it back anyway (see notes above) */
                                /* but the thrown-back clause has no extra selectivity */
-                               rinfo->this_selec = 1.0;
+                               rinfo->this_selec = 2.0;
                                distribute_restrictinfo_to_rels(root, rinfo);
                        }
                        else
@@ -1220,7 +1220,7 @@ reconsider_outer_join_clauses(PlannerInfo *root)
                                        list_delete_cell(root->right_join_clauses, cell, prev);
                                /* we throw it back anyway (see notes above) */
                                /* but the thrown-back clause has no extra selectivity */
-                               rinfo->this_selec = 1.0;
+                               rinfo->this_selec = 2.0;
                                distribute_restrictinfo_to_rels(root, rinfo);
                        }
                        else
@@ -1242,7 +1242,7 @@ reconsider_outer_join_clauses(PlannerInfo *root)
                                        list_delete_cell(root->full_join_clauses, cell, prev);
                                /* we throw it back anyway (see notes above) */
                                /* but the thrown-back clause has no extra selectivity */
-                               rinfo->this_selec = 1.0;
+                               rinfo->this_selec = 2.0;
                                distribute_restrictinfo_to_rels(root, rinfo);
                        }
                        else

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 9420bf49f58a8eab50039b7dc3c2c39a7ce29fcd..e0ef748bff19de0b3016d2c6bb889c6ee17edc20 100644 (file)
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -991,7 +991,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 NIL */
        List       *mergeopfamilies;    /* opfamilies containing clause operator */