Reduce "X = X" to "X IS NOT NULL", if it's easy to do so.

If the operator is a strict btree equality operator, and X isn't volatile,
then the clause must yield true for any non-null value of X, or null if X
is null.  At top level of a WHERE clause, we can ignore the distinction
between false and null results, so it's valid to simplify the clause to
"X IS NOT NULL".  This is a useful improvement mainly because we'll get
a far better selectivity estimate in most cases.

Because such cases seldom arise in well-written queries, it is unappetizing
to expend a lot of planner cycles looking for them ... but it turns out
that there's a place we can shoehorn this in practically for free, because
equivclass.c already has to detect and reject candidate equivalences of the
form X = X.  That doesn't catch every place that it would be valid to
simplify to X IS NOT NULL, but it catches the typical case.  Working harder
doesn't seem justified.

Patch by me, reviewed by Petr Jelinek

Discussion: https://postgr.es/m/CAMjNa7cC4X9YR-vAJS-jSYCajhRDvJQnN7m2sLH1wLh-_Z2bsw@mail.gmail.com

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 7997f50c18d6fbf4d9db40b5dc50b49413ec1601..a225414c97017aeded3b07bd6b0da6babb1009eb 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -27,6 +27,7 @@
 #include "optimizer/paths.h"
 #include "optimizer/planmain.h"
 #include "optimizer/prep.h"
+#include "optimizer/restrictinfo.h"
 #include "optimizer/var.h"
 #include "utils/lsyscache.h"
 
@@ -71,8 +72,14 @@ static bool reconsider_full_join_clause(PlannerInfo *root,
  *       any delay by an outer join, so its two sides can be considered equal
  *       anywhere they are both computable; moreover that equality can be
  *       extended transitively.  Record this knowledge in the EquivalenceClass
- *       data structure.  Returns TRUE if successful, FALSE if not (in which
- *       case caller should treat the clause as ordinary, not an equivalence).
+ *       data structure, if applicable.  Returns TRUE if successful, FALSE if not
+ *       (in which case caller should treat the clause as ordinary, not an
+ *       equivalence).
+ *
+ * In some cases, although we cannot convert a clause into EquivalenceClass
+ * knowledge, we can still modify it to a more useful form than the original.
+ * Then, *p_restrictinfo will be replaced by a new RestrictInfo, which is what
+ * the caller should use for further processing.
  *
  * If below_outer_join is true, then the clause was found below the nullable
  * side of an outer join, so its sides might validly be both NULL rather than
@@ -104,9 +111,11 @@ static bool reconsider_full_join_clause(PlannerInfo *root,
  * memory context.
  */
 bool
-process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo,
+process_equivalence(PlannerInfo *root,
+                                       RestrictInfo **p_restrictinfo,
                                        bool below_outer_join)
 {
+       RestrictInfo *restrictinfo = *p_restrictinfo;
        Expr       *clause = restrictinfo->clause;
        Oid                     opno,
                                collation,
@@ -154,16 +163,45 @@ process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo,
                                                                           collation);
 
        /*
-        * 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.
+        * Clauses of the form X=X cannot be translated into EquivalenceClasses.
+        * We'd either end up with a single-entry EC, losing the knowledge that
+        * the clause was present at all, or else make an EC with duplicate
+        * entries, causing other issues.
         */
        if (equal(item1, item2))
-               return false;                   /* X=X is not a useful equivalence */
+       {
+               /*
+                * If the operator is strict, then the clause can be treated as just
+                * "X IS NOT NULL".  (Since we know we are considering a top-level
+                * qual, we can ignore the difference between FALSE and NULL results.)
+                * It's worth making the conversion because we'll typically get a much
+                * better selectivity estimate than we would for X=X.
+                *
+                * If the operator is not strict, we can't be sure what it will do
+                * with NULLs, so don't attempt to optimize it.
+                */
+               set_opfuncid((OpExpr *) clause);
+               if (func_strict(((OpExpr *) clause)->opfuncid))
+               {
+                       NullTest   *ntest = makeNode(NullTest);
+
+                       ntest->arg = item1;
+                       ntest->nulltesttype = IS_NOT_NULL;
+                       ntest->argisrow = false;        /* correct even if composite arg */
+                       ntest->location = -1;
+
+                       *p_restrictinfo =
+                               make_restrictinfo((Expr *) ntest,
+                                                                 restrictinfo->is_pushed_down,
+                                                                 restrictinfo->outerjoin_delayed,
+                                                                 restrictinfo->pseudoconstant,
+                                                                 restrictinfo->security_level,
+                                                                 NULL,
+                                                                 restrictinfo->outer_relids,
+                                                                 restrictinfo->nullable_relids);
+               }
+               return false;
+       }
 
        /*
         * If below outer join, check for strictness, else reject.
@@ -1741,7 +1779,7 @@ reconsider_outer_join_clause(PlannerInfo *root, RestrictInfo *rinfo,
                                                                                                   bms_copy(inner_relids),
                                                                                                   bms_copy(inner_nullable_relids),
                                                                                                   cur_ec->ec_min_security);
-                       if (process_equivalence(root, newrinfo, true))
+                       if (process_equivalence(root, &newrinfo, true))
                                match = true;
                }
 
@@ -1884,7 +1922,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
                                                                                                           bms_copy(left_relids),
                                                                                                           bms_copy(left_nullable_relids),
                                                                                                           cur_ec->ec_min_security);
-                               if (process_equivalence(root, newrinfo, true))
+                               if (process_equivalence(root, &newrinfo, true))
                                        matchleft = true;
                        }
                        eq_op = select_equality_operator(cur_ec,
@@ -1899,7 +1937,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
                                                                                                           bms_copy(right_relids),
                                                                                                           bms_copy(right_nullable_relids),
                                                                                                           cur_ec->ec_min_security);
-                               if (process_equivalence(root, newrinfo, true))
+                               if (process_equivalence(root, &newrinfo, true))
                                        matchright = true;
                        }
                }

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 9931dddba43a6675d39ad5a621335166cd8a779e..974eb58d83751bc2907556555e42be01710f40cb 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -1964,10 +1964,11 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
                if (maybe_equivalence)
                {
                        if (check_equivalence_delay(root, restrictinfo) &&
-                               process_equivalence(root, restrictinfo, below_outer_join))
+                               process_equivalence(root, &restrictinfo, below_outer_join))
                                return;
                        /* EC rejected it, so set left_ec/right_ec the hard way ... */
-                       initialize_mergeclause_eclasses(root, restrictinfo);
+                       if (restrictinfo->mergeopfamilies)      /* EC might have changed this */
+                               initialize_mergeclause_eclasses(root, restrictinfo);
                        /* ... and fall through to distribute_restrictinfo_to_rels */
                }
                else if (maybe_outer_join && restrictinfo->can_join)

diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index a15eee54bb8a1e655817df1e27a1ae9fbc40cf09..ea886b6501bfa6a503b7ebf6f3d78a7d2c0ec54a 100644 (file)
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -127,7 +127,8 @@ typedef bool (*ec_matches_callback_type) (PlannerInfo *root,
                                                                                  EquivalenceMember *em,
                                                                                  void *arg);
 
-extern bool process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo,
+extern bool process_equivalence(PlannerInfo *root,
+                                       RestrictInfo **p_restrictinfo,
                                        bool below_outer_join);
 extern Expr *canonicalize_ec_expression(Expr *expr,
                                                   Oid req_type, Oid req_collation);

diff --git a/src/test/regress/expected/equivclass.out b/src/test/regress/expected/equivclass.out
index a96b2a1b07c36bfa0c9888c9e30631134fe68637..c448d85dec37ef1ce5b66f6903833f79ffaba934 100644 (file)
--- a/src/test/regress/expected/equivclass.out
+++ b/src/test/regress/expected/equivclass.out
@@ -421,3 +421,21 @@ reset session authorization;
 revoke select on ec0 from regress_user_ectest;
 revoke select on ec1 from regress_user_ectest;
 drop user regress_user_ectest;
+-- check that X=X is converted to X IS NOT NULL when appropriate
+explain (costs off)
+  select * from tenk1 where unique1 = unique1 and unique2 = unique2;
+                         QUERY PLAN                          
+-------------------------------------------------------------
+ Seq Scan on tenk1
+   Filter: ((unique1 IS NOT NULL) AND (unique2 IS NOT NULL))
+(2 rows)
+
+-- this could be converted, but isn't at present
+explain (costs off)
+  select * from tenk1 where unique1 = unique1 or unique2 = unique2;
+                       QUERY PLAN                       
+--------------------------------------------------------
+ Seq Scan on tenk1
+   Filter: ((unique1 = unique1) OR (unique2 = unique2))
+(2 rows)
+

diff --git a/src/test/regress/sql/equivclass.sql b/src/test/regress/sql/equivclass.sql
index 0e4aa0cd2c55c69662bcf6c9ec9beedfd04c6108..85aa65de392e46898388515209d249a45419b702 100644 (file)
--- a/src/test/regress/sql/equivclass.sql
+++ b/src/test/regress/sql/equivclass.sql
@@ -254,3 +254,11 @@ revoke select on ec0 from regress_user_ectest;
 revoke select on ec1 from regress_user_ectest;
 
 drop user regress_user_ectest;
+
+-- check that X=X is converted to X IS NOT NULL when appropriate
+explain (costs off)
+  select * from tenk1 where unique1 = unique1 and unique2 = unique2;
+
+-- this could be converted, but isn't at present
+explain (costs off)
+  select * from tenk1 where unique1 = unique1 or unique2 = unique2;