From: Tom Lane Date: Mon, 1 Dec 2008 21:06:20 +0000 (+0000) Subject: Fix an oversight in the code that makes transitive-equality deductions from X-Git-Tag: REL8_3_6~32 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=07531c6c37b6c9e3e26c379c4883b1459055bbbe;p=postgresql 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 c54cbc9d02..6bc2595eaf 100644 --- 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.322 2008/01/09 08:46:44 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.322.2.1 2008/12/01 21:06:20 tgl Exp $ * * NOTES * Every node type that can appear in stored rules' parsetrees *must* @@ -1459,6 +1459,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 e358c6990e..072e527085 100644 --- 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.90 2008/01/11 17:00:45 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/clausesel.c,v 1.90.2.1 2008/12/01 21:06:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -453,6 +453,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 0e36af637f..13ed31a5c9 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -10,7 +10,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.9.2.1 2008/03/31 16:59:33 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.9.2.2 2008/12/01 21:06:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1197,7 +1197,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 @@ -1219,7 +1219,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 @@ -1241,7 +1241,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 56ff0f3f2f..3d4b9ddcf8 100644 --- 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.154.2.2 2008/04/21 20:54:24 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.154.2.3 2008/12/01 21:06:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -972,7 +972,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 */