Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit df62977

Browse files
committed
Fix an old error in clause_selectivity: the default selectivity estimate
for unhandled clause types ought to be 0.5, not 1.0. I fear I introduced this silliness due to misreading the intent of the very-poorly-structured code that was there when we inherited the file from Berkeley. The lack of sanity in this behavior was exposed by an example from Sim Zacks. (Arguably this is a bug fix and should be back-patched, but I'm a bit hesitant to introduce a possible planner behavior change in the back branches; it might detune queries that worked acceptably in the past.) While at it, make estimation for DistinctExpr do something marginally realistic, rather than just defaulting.
1 parent f3e3f2e commit df62977

File tree

1 file changed

+17
-13
lines changed

1 file changed

+17
-13
lines changed

src/backend/optimizer/path/clausesel.c

+17-13
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.89 2008/01/01 19:45:50 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/clausesel.c,v 1.90 2008/01/11 17:00:45 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -428,7 +428,7 @@ clause_selectivity(PlannerInfo *root,
428428
int varRelid,
429429
JoinType jointype)
430430
{
431-
Selectivity s1 = 1.0; /* default for any unhandled clause type */
431+
Selectivity s1 = 0.5; /* default for any unhandled clause type */
432432
RestrictInfo *rinfo = NULL;
433433
bool cacheable = false;
434434

@@ -450,7 +450,7 @@ clause_selectivity(PlannerInfo *root,
450450
if (rinfo->pseudoconstant)
451451
{
452452
if (!IsA(rinfo->clause, Const))
453-
return s1;
453+
return (Selectivity) 1.0;
454454
}
455455

456456
/*
@@ -517,9 +517,8 @@ clause_selectivity(PlannerInfo *root,
517517
{
518518
/*
519519
* XXX not smart about subquery references... any way to do
520-
* better?
520+
* better than default?
521521
*/
522-
s1 = 0.5;
523522
}
524523
else
525524
{
@@ -560,8 +559,7 @@ clause_selectivity(PlannerInfo *root,
560559
}
561560
else
562561
{
563-
/* XXX any way to do better? */
564-
s1 = (Selectivity) 0.5;
562+
/* XXX any way to do better than default? */
565563
}
566564
}
567565
else if (not_clause(clause))
@@ -601,7 +599,7 @@ clause_selectivity(PlannerInfo *root,
601599
s1 = s1 + s2 - s1 * s2;
602600
}
603601
}
604-
else if (is_opclause(clause))
602+
else if (is_opclause(clause) || IsA(clause, DistinctExpr))
605603
{
606604
Oid opno = ((OpExpr *) clause)->opno;
607605
bool is_join_clause;
@@ -642,6 +640,15 @@ clause_selectivity(PlannerInfo *root,
642640
((OpExpr *) clause)->args,
643641
varRelid);
644642
}
643+
644+
/*
645+
* DistinctExpr has the same representation as OpExpr, but the
646+
* contained operator is "=" not "<>", so we must negate the result.
647+
* This estimation method doesn't give the right behavior for nulls,
648+
* but it's better than doing nothing.
649+
*/
650+
if (IsA(clause, DistinctExpr))
651+
s1 = 1.0 - s1;
645652
}
646653
else if (is_funcclause(clause))
647654
{
@@ -652,18 +659,15 @@ clause_selectivity(PlannerInfo *root,
652659
*/
653660
s1 = (Selectivity) 0.3333333;
654661
}
662+
#ifdef NOT_USED
655663
else if (is_subplan(clause))
656664
{
657665
/*
658666
* Just for the moment! FIX ME! - vadim 02/04/98
659667
*/
660668
s1 = (Selectivity) 0.5;
661669
}
662-
else if (IsA(clause, DistinctExpr))
663-
{
664-
/* can we do better? */
665-
s1 = (Selectivity) 0.5;
666-
}
670+
#endif
667671
else if (IsA(clause, ScalarArrayOpExpr))
668672
{
669673
/* First, decide if it's a join clause, same as for OpExpr */

0 commit comments

Comments
 (0)