Petr Jelinek <petr.jeli...@2ndquadrant.com> writes:
> On 07/10/17 04:19, Tom Lane wrote:
>> (edit: a few minutes later, I seem to remember that equivclass.c has
>> to do something special with the X=X case, so maybe it could do
>> something else special instead, with little new overhead.)
> So I wrote prototype of achieving this optimization and it seems to be
> really quite simple code-wise (see attached). I did only a limited
> manual testing of this but I don't see any negative impact on planning time.
No, I'm afraid you didn't read that comment closely enough. This will
flat out fail for cases like "select ... where x=x order by x", because
there will already be a single-element EC for x and so the clause will
just disappear entirely. If that doesn't happen, then instead you're
creating an EC with duplicate entries, which is an idea I seriously
dislike; the semantics of that don't seem clear at all to me.
What I was imagining was that having detected X=X, instead of "throwing
back" the clause as-is we could throw back an X IS NOT NULL clause,
along the lines of the attached.
This passes the smell test for me in the sense of not adding any
significant number of planner cycles except when the weird case occurs.
It leaves something on the table in that there are some situations
where X=X could be converted, but we won't do it because we don't see
the clause as being a potential EC (because it's not at top level),
as in the second new regression test case below. I think that's
probably all right; I don't see any way to be more thorough without
adding a lot of new cycles all the time, and I don't believe this is
worth that.
regards, tom lane
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 7997f50..a225414 100644
*** a/src/backend/optimizer/path/equivclass.c
--- b/src/backend/optimizer/path/equivclass.c
***************
*** 27,32 ****
--- 27,33 ----
#include "optimizer/paths.h"
#include "optimizer/planmain.h"
#include "optimizer/prep.h"
+ #include "optimizer/restrictinfo.h"
#include "optimizer/var.h"
#include "utils/lsyscache.h"
*************** static bool reconsider_full_join_clause(
*** 71,78 ****
* 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).
*
* 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
--- 72,85 ----
* 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, 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
*************** static bool reconsider_full_join_clause(
*** 104,112 ****
* memory context.
*/
bool
! process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo,
bool below_outer_join)
{
Expr *clause = restrictinfo->clause;
Oid opno,
collation,
--- 111,121 ----
* memory context.
*/
bool
! process_equivalence(PlannerInfo *root,
! RestrictInfo **p_restrictinfo,
bool below_outer_join)
{
+ RestrictInfo *restrictinfo = *p_restrictinfo;
Expr *clause = restrictinfo->clause;
Oid opno,
collation,
*************** process_equivalence(PlannerInfo *root, R
*** 154,169 ****
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.
*/
if (equal(item1, item2))
! return false; /* X=X is not a useful equivalence */
/*
* If below outer join, check for strictness, else reject.
--- 163,207 ----
collation);
/*
! * 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))
! {
! /*
! * 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.
*************** reconsider_outer_join_clause(PlannerInfo
*** 1741,1747 ****
bms_copy(inner_relids),
bms_copy(inner_nullable_relids),
cur_ec->ec_min_security);
! if (process_equivalence(root, newrinfo, true))
match = true;
}
--- 1779,1785 ----
bms_copy(inner_relids),
bms_copy(inner_nullable_relids),
cur_ec->ec_min_security);
! if (process_equivalence(root, &newrinfo, true))
match = true;
}
*************** reconsider_full_join_clause(PlannerInfo
*** 1884,1890 ****
bms_copy(left_relids),
bms_copy(left_nullable_relids),
cur_ec->ec_min_security);
! if (process_equivalence(root, newrinfo, true))
matchleft = true;
}
eq_op = select_equality_operator(cur_ec,
--- 1922,1928 ----
bms_copy(left_relids),
bms_copy(left_nullable_relids),
cur_ec->ec_min_security);
! if (process_equivalence(root, &newrinfo, true))
matchleft = true;
}
eq_op = select_equality_operator(cur_ec,
*************** reconsider_full_join_clause(PlannerInfo
*** 1899,1905 ****
bms_copy(right_relids),
bms_copy(right_nullable_relids),
cur_ec->ec_min_security);
! if (process_equivalence(root, newrinfo, true))
matchright = true;
}
}
--- 1937,1943 ----
bms_copy(right_relids),
bms_copy(right_nullable_relids),
cur_ec->ec_min_security);
! 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 9931ddd..974eb58 100644
*** a/src/backend/optimizer/plan/initsplan.c
--- b/src/backend/optimizer/plan/initsplan.c
*************** distribute_qual_to_rels(PlannerInfo *roo
*** 1964,1973 ****
if (maybe_equivalence)
{
if (check_equivalence_delay(root, restrictinfo) &&
! 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);
/* ... and fall through to distribute_restrictinfo_to_rels */
}
else if (maybe_outer_join && restrictinfo->can_join)
--- 1964,1974 ----
if (maybe_equivalence)
{
if (check_equivalence_delay(root, restrictinfo) &&
! process_equivalence(root, &restrictinfo, below_outer_join))
return;
/* EC rejected it, so set left_ec/right_ec the hard way ... */
! 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 a15eee5..ea886b6 100644
*** a/src/include/optimizer/paths.h
--- b/src/include/optimizer/paths.h
*************** typedef bool (*ec_matches_callback_type)
*** 127,133 ****
EquivalenceMember *em,
void *arg);
! extern bool process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo,
bool below_outer_join);
extern Expr *canonicalize_ec_expression(Expr *expr,
Oid req_type, Oid req_collation);
--- 127,134 ----
EquivalenceMember *em,
void *arg);
! 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 a96b2a1..c448d85 100644
*** a/src/test/regress/expected/equivclass.out
--- b/src/test/regress/expected/equivclass.out
*************** reset session authorization;
*** 421,423 ****
--- 421,441 ----
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 0e4aa0c..85aa65d 100644
*** a/src/test/regress/sql/equivclass.sql
--- b/src/test/regress/sql/equivclass.sql
*************** revoke select on ec0 from regress_user_e
*** 254,256 ****
--- 254,264 ----
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;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
