Skip to content

Commit a1feb90

Browse files
tglsfdctglsfdc
committed
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.
1 parent 7fb2753 commit a1feb90

File tree

4 files changed

+16
-8
lines changed

4 files changed

+16
-8
lines changed

‎src/backend/nodes/outfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.345 2008/11/15 19:43:46 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.346 2008/12/01 21:06:12 tgl Exp $
1212
*
1313
* NOTES
1414
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1563,6 +1563,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
15631563
WRITE_BITMAPSET_FIELD(right_relids);
15641564
WRITE_NODE_FIELD(orclause);
15651565
/* don't write parent_ec, leads to infinite recursion in plan tree dump */
1566+
WRITE_FLOAT_FIELD(this_selec, "%.4f");
15661567
WRITE_NODE_FIELD(mergeopfamilies);
15671568
/* don't write left_ec, leads to infinite recursion in plan tree dump */
15681569
/* don't write right_ec, leads to infinite recursion in plan tree dump */

‎src/backend/optimizer/path/clausesel.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/clausesel.c,v 1.94 2008/10/04 21:56:53 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/clausesel.c,v 1.95 2008/12/01 21:06:13 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -513,6 +513,12 @@ clause_selectivity(PlannerInfo *root,
513513
return (Selectivity) 1.0;
514514
}
515515

516+
/*
517+
* If the clause is marked redundant, always return 1.0.
518+
*/
519+
if (rinfo->this_selec > 1)
520+
return (Selectivity) 1.0;
521+
516522
/*
517523
* If possible, cache the result of the selectivity calculation for
518524
* the clause. We can cache if varRelid is zero or the clause

‎src/backend/optimizer/path/equivclass.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Portions Copyright (c) 1994, Regents of the University of California
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.13 2008/10/21 20:42:52 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.14 2008/12/01 21:06:13 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -1198,7 +1198,7 @@ reconsider_outer_join_clauses(PlannerInfo *root)
11981198
list_delete_cell(root->left_join_clauses, cell, prev);
11991199
/* we throw it back anyway (see notes above) */
12001200
/* but the thrown-back clause has no extra selectivity */
1201-
rinfo->this_selec = 1.0;
1201+
rinfo->this_selec = 2.0;
12021202
distribute_restrictinfo_to_rels(root, rinfo);
12031203
}
12041204
else
@@ -1220,7 +1220,7 @@ reconsider_outer_join_clauses(PlannerInfo *root)
12201220
list_delete_cell(root->right_join_clauses, cell, prev);
12211221
/* we throw it back anyway (see notes above) */
12221222
/* but the thrown-back clause has no extra selectivity */
1223-
rinfo->this_selec = 1.0;
1223+
rinfo->this_selec = 2.0;
12241224
distribute_restrictinfo_to_rels(root, rinfo);
12251225
}
12261226
else
@@ -1242,7 +1242,7 @@ reconsider_outer_join_clauses(PlannerInfo *root)
12421242
list_delete_cell(root->full_join_clauses, cell, prev);
12431243
/* we throw it back anyway (see notes above) */
12441244
/* but the thrown-back clause has no extra selectivity */
1245-
rinfo->this_selec = 1.0;
1245+
rinfo->this_selec = 2.0;
12461246
distribute_restrictinfo_to_rels(root, rinfo);
12471247
}
12481248
else

‎src/include/nodes/relation.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.164 2008/11/11 18:13:32 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.165 2008/12/01 21:06:13 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -991,7 +991,8 @@ typedef struct RestrictInfo
991991

992992
/* cache space for cost and selectivity */
993993
QualCost eval_cost; /* eval cost of clause; -1 if not yet set */
994-
Selectivity this_selec; /* selectivity; -1 if not yet set */
994+
Selectivity this_selec; /* selectivity; -1 if not yet set; >1 means
995+
* a redundant clause */
995996

996997
/* valid if clause is mergejoinable, else NIL */
997998
List *mergeopfamilies; /* opfamilies containing clause operator */

0 commit comments

Comments
 (0)