From: Tom Lane Date: Mon, 1 Dec 2008 21:06:31 +0000 (+0000) Subject: Fix an oversight in the code that makes transitive-equality deductions from X-Git-Tag: REL8_2_12~19 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4914b708471594230caadb8cf2cbb621f2e8c063;p=thirdparty%2Fpostgresql.git 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 ac6787688d7..b1080433e79 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.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 463408f26fd..ccab70796e6 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.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 abc50ef14d0..6550ee4f241 100644 --- 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 865c7caff66..e6bae2ada05 100644 --- 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 */