diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README index dc95ac5c98e..f862526a9e5 100644 --- a/src/backend/optimizer/README +++ b/src/backend/optimizer/README @@ -1,4 +1,4 @@ -$PostgreSQL: pgsql/src/backend/optimizer/README,v 1.49.2.1 2009/07/21 02:02:51 tgl Exp $ +$PostgreSQL: pgsql/src/backend/optimizer/README,v 1.49.2.2 2009/09/29 01:20:53 tgl Exp $ Optimizer ========= @@ -480,7 +480,10 @@ operator is mergejoinable, so there is no way for a volatile expression to get into EquivalenceClasses otherwise. Aggregates are disallowed in WHERE altogether, so will never be found in a mergejoinable clause.) This is just a convenience to maintain a uniform PathKey representation: such an -EquivalenceClass will never be merged with any other. +EquivalenceClass will never be merged with any other. Note in particular +that a single-item EquivalenceClass {a.x} is *not* meant to imply an +assertion that a.x = a.x; the practical effect of this is that a.x could +be NULL. An EquivalenceClass also contains a list of btree opfamily OIDs, which determines what the equalities it represents actually "mean". All the diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 5c07a103ad9..3c89a69c46b 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.19.2.1 2009/09/12 00:05:07 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.19.2.2 2009/09/29 01:20:54 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -114,6 +114,19 @@ process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo, item1_relids = restrictinfo->left_relids; item2_relids = restrictinfo->right_relids; + /* + * Reject clauses of the form X=X. These are not as redundant as they + * might seem at first glance: assuming the operator is strict, this is + * really an expensive way to write X IS NOT NULL. So we must not risk + * just losing the clause, which would be possible if there is already + * a single-element EquivalenceClass containing X. The case is not + * common enough to be worth contorting the EC machinery for, so just + * reject the clause and let it be processed as a normal restriction + * clause. + */ + if (equal(item1, item2)) + return false; /* X=X is not a useful equivalence */ + /* * If below outer join, check for strictness, else reject. */ @@ -152,13 +165,10 @@ process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo, * * 4. We find neither. Make a new, two-entry EC. * - * Note: since all ECs are built through this process, it's impossible - * that we'd match an item in more than one existing EC. It is possible - * to match more than once within an EC, if someone fed us something silly - * like "WHERE X=X". (However, we can't simply discard such clauses, - * since they should fail when X is null; so we will build a 2-member EC - * to ensure the correct restriction clause gets generated. Hence there - * is no shortcut here for item1 and item2 equal.) + * Note: since all ECs are built through this process or the similar + * search in get_eclass_for_sort_expr(), it's impossible that we'd match + * an item in more than one existing nonvolatile EC. So it's okay to stop + * at the first match. */ ec1 = ec2 = NULL; em1 = em2 = NULL; diff --git a/src/test/regress/expected/select.out b/src/test/regress/expected/select.out index 47c3e67b230..449341739c9 100644 --- a/src/test/regress/expected/select.out +++ b/src/test/regress/expected/select.out @@ -768,3 +768,19 @@ select sillysrf(-1) order by 1; (4 rows) drop function sillysrf(int); +-- X = X isn't a no-op, it's effectively X IS NOT NULL assuming = is strict +-- (see bug #5084) +select * from (values (2),(null),(1)) v(k) where k = k order by k; + k +--- + 1 + 2 +(2 rows) + +select * from (values (2),(null),(1)) v(k) where k = k; + k +--- + 2 + 1 +(2 rows) + diff --git a/src/test/regress/sql/select.sql b/src/test/regress/sql/select.sql index a9ddd5e5861..451fcf78d9e 100644 --- a/src/test/regress/sql/select.sql +++ b/src/test/regress/sql/select.sql @@ -202,3 +202,8 @@ select sillysrf(42); select sillysrf(-1) order by 1; drop function sillysrf(int); + +-- X = X isn't a no-op, it's effectively X IS NOT NULL assuming = is strict +-- (see bug #5084) +select * from (values (2),(null),(1)) v(k) where k = k order by k; +select * from (values (2),(null),(1)) v(k) where k = k;