diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README index a0df388993b525962093a0a26ebc4405decb2832..de04bfa81e46082062070342eeffa0e96b817874 100644 --- a/src/backend/optimizer/README +++ b/src/backend/optimizer/README @@ -467,7 +467,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 13ed31a5c96e9d94daefd208b38309f799e6f0dd..56e93c2c41c6e3b8051b4a36f0b447054a1e6578 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.2 2008/12/01 21:06:20 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/equivclass.c,v 1.9.2.3 2009/09/29 01:21:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -113,6 +113,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. */ @@ -151,13 +164,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 2936f0306aba92ea92d6f12a59c66a01b195d785..6465317231aa4c9aa299466e4bccab92cc139565 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 bb1fc9b9347d0e9db40550aa9cd01a4721f1b984..3bda45a2fbb83782be0f9452d3db12742944459a 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;