Hi, In good written query IS NULL and IS NOT NULL check on primary and non null constraints columns should not happen but if it is mentioned PostgreSQL have to be smart enough for not checking every return result about null value on primary key column. Instead it can be evaluate its truth value and set the result only once. The attached patch evaluate and set the truth value for null and not null check on primary column on planning time if the relation attribute is not mention on nullable side of outer join.
Thought? regards Surafel
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 750586fceb..417758f705 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -22,6 +22,7 @@ #include "access/htup_details.h" #include "catalog/pg_aggregate.h" #include "catalog/pg_class.h" +#include "catalog/pg_constraint.h" #include "catalog/pg_language.h" #include "catalog/pg_operator.h" #include "catalog/pg_proc.h" @@ -42,6 +43,7 @@ #include "parser/parse_agg.h" #include "parser/parse_coerce.h" #include "parser/parse_func.h" +#include "parser/parsetree.h" #include "rewrite/rewriteManip.h" #include "tcop/tcopprot.h" #include "utils/acl.h" @@ -90,6 +92,14 @@ typedef struct char *prosrc; } inline_error_callback_arg; +typedef struct check_null_side_state +{ + Relids relids; /* base relids within this subtree */ + bool contains_outer; /* does subtree contain outer join(s)? */ + JoinType jointype; /* type of join */ + List *sub_states; /* List of states for subtree components */ +} check_null_side_state; + typedef struct { char max_hazard; /* worst proparallel hazard found so far */ @@ -156,6 +166,9 @@ static Query *substitute_actual_srf_parameters(Query *expr, int nargs, List *args); static Node *substitute_actual_srf_parameters_mutator(Node *node, substitute_actual_srf_parameters_context *context); +static bool check_null_side(PlannerInfo *root, int relid); +static check_null_side_state * collect_jointree_data(Node *jtnode); + /***************************************************************************** @@ -2245,7 +2258,6 @@ rowtype_field_matches(Oid rowtypeid, int fieldnum, return true; } - /*-------------------- * eval_const_expressions * @@ -2626,6 +2638,7 @@ eval_const_expressions_mutator(Node *node, { has_null_input |= ((Const *) lfirst(arg))->constisnull; all_null_input &= ((Const *) lfirst(arg))->constisnull; + } else has_nonconst_input = true; @@ -3382,7 +3395,47 @@ eval_const_expressions_mutator(Node *node, return makeBoolConst(result, false); } + if (IsA(arg, Var) && + ((Var *) arg)->varlevelsup == 0 && context->root) + { + /* + * Evaluate the test if it is on primary column and the + * relation is not mentioned on nullable side of outer + * join + */ + Var *var = (Var *) arg; + Query *parse = context->root->parse; + int relid; + Bitmapset *pkattnos; + Oid constraintOid; + RangeTblEntry *rte; + relid = var->varno; + rte = rt_fetch(relid, parse->rtable); + pkattnos = get_primary_key_attnos(rte->relid, false, &constraintOid); + + if (pkattnos != NULL && bms_is_member(var->varattno - FirstLowInvalidHeapAttributeNumber, pkattnos) + && !check_null_side(context->root, relid)) + { + bool result; + + switch (ntest->nulltesttype) + { + case IS_NULL: + result = false; + break; + case IS_NOT_NULL: + result = true; + break; + default: + elog(ERROR, "unrecognized nulltesttype: %d", + (int) ntest->nulltesttype); + result = false; /* keep compiler quiet */ + break; + } + return makeBoolConst(result, false); + } + } newntest = makeNode(NullTest); newntest->arg = (Expr *) arg; newntest->nulltesttype = ntest->nulltesttype; @@ -3572,6 +3625,118 @@ eval_const_expressions_mutator(Node *node, return ece_generic_processing(node); } +/* + * Check a relation attributes on nullable side of the outer join. + */ +static bool +check_null_side(PlannerInfo *root, int relid) +{ + check_null_side_state *state; + ListCell *l; + + state = collect_jointree_data((Node *) root->parse->jointree); + + /* if no outer joins the relation is not on nullable side */ + if (state == NULL || !state->contains_outer) + return false; + + /* scan the state and check relation on nullable outer join side */ + foreach(l, state->sub_states) + { + check_null_side_state *sub_state = (check_null_side_state *) lfirst(l); + + if (sub_state->contains_outer) + { + if (sub_state->jointype == JOIN_LEFT) + { + check_null_side_state *right_state = (check_null_side_state *) lsecond(sub_state->sub_states); + + if (bms_is_member(relid, right_state->relids)) + return true; + } + if (sub_state->jointype == JOIN_RIGHT) + { + check_null_side_state *left_state = (check_null_side_state *) linitial(sub_state->sub_states); + + if (bms_is_member(relid, left_state->relids)) + return true; + } + } + } + + return false; +} + + +/* + * Collecting data about join. this function is similar to reduce_outer_joins_pass1 + * + * Returns a state node describing the given jointree node. + */ +static check_null_side_state * +collect_jointree_data(Node *jtnode) +{ + check_null_side_state *result; + + result = (check_null_side_state *) + palloc(sizeof(check_null_side_state)); + result->relids = NULL; + result->contains_outer = false; + result->sub_states = NIL; + + if (jtnode == NULL) + return result; + if (IsA(jtnode, RangeTblRef)) + { + int varno = ((RangeTblRef *) jtnode)->rtindex; + + result->relids = bms_make_singleton(varno); + } + else if (IsA(jtnode, FromExpr)) + { + FromExpr *f = (FromExpr *) jtnode; + ListCell *l; + + foreach(l, f->fromlist) + { + check_null_side_state *sub_state; + + sub_state = collect_jointree_data(lfirst(l)); + result->relids = bms_add_members(result->relids, + sub_state->relids); + result->contains_outer |= sub_state->contains_outer; + result->sub_states = lappend(result->sub_states, sub_state); + } + } + else if (IsA(jtnode, JoinExpr)) + { + JoinExpr *j = (JoinExpr *) jtnode; + check_null_side_state *sub_state; + + /* join's own RT index is not wanted in result->relids */ + if (j->jointype == JOIN_RIGHT || j->jointype == JOIN_LEFT) + result->contains_outer = true; + + sub_state = collect_jointree_data(j->larg); + result->jointype = j->jointype; + result->relids = bms_add_members(result->relids, + sub_state->relids); + sub_state->contains_outer |= sub_state->contains_outer; + result->sub_states = lappend(result->sub_states, sub_state); + + sub_state = collect_jointree_data(j->rarg); + result->jointype = j->jointype; + result->relids = bms_add_members(result->relids, + sub_state->relids); + result->contains_outer |= sub_state->contains_outer; + result->sub_states = lappend(result->sub_states, sub_state); + } + else + elog(ERROR, "unrecognized node type: %d", + (int) nodeTag(jtnode)); + return result; +} + /* * Subroutine for eval_const_expressions: check for non-Const nodes. * diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index a46b1573bd..a42f58c9f8 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4577,6 +4577,32 @@ SELECT b.* FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0 1 | (1 row) +explain (costs off) +SELECT * FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0); + QUERY PLAN +------------------------------------------ + Hash Left Join + Hash Cond: (b.a_id = a.id) + Filter: ((a.id IS NULL) OR (a.id > 0)) + -> Seq Scan on b + -> Hash + -> Seq Scan on a +(6 rows) + +explain (costs off) +SELECT * FROM b RIGHT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0); + QUERY PLAN +----------------------------------------------- + Hash Right Join + Hash Cond: (b.a_id = a.id) + -> Seq Scan on b + -> Hash + -> Bitmap Heap Scan on a + Recheck Cond: (id > 0) + -> Bitmap Index Scan on a_pkey + Index Cond: (id > 0) +(8 rows) + rollback; -- another join removal bug: this is not optimizable, either begin; diff --git a/src/test/regress/expected/select.out b/src/test/regress/expected/select.out index c441049f41..1514b5a05a 100644 --- a/src/test/regress/expected/select.out +++ b/src/test/regress/expected/select.out @@ -903,6 +903,22 @@ select unique1, unique2 from onek2 0 | 998 (2 rows) +CREATE TEMP TABLE a (id int PRIMARY KEY); +explain (costs off) +SELECT * FROM a WHERE id IS NULL; + QUERY PLAN +-------------------------- + Result + One-Time Filter: false +(2 rows) + +explain (costs off) +SELECT * FROM a WHERE id IS NOT NULL; + QUERY PLAN +--------------- + Seq Scan on a +(1 row) + -- -- Test some corner cases that have been known to confuse the planner -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 1403e0ffe7..4146562c3b 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1589,6 +1589,11 @@ INSERT INTO b VALUES (0, 0), (1, NULL); SELECT * FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0); SELECT b.* FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0); +explain (costs off) +SELECT * FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0); + +explain (costs off) +SELECT * FROM b RIGHT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0); rollback; -- another join removal bug: this is not optimizable, either diff --git a/src/test/regress/sql/select.sql b/src/test/regress/sql/select.sql index b5929b2eca..6c4f4f1af8 100644 --- a/src/test/regress/sql/select.sql +++ b/src/test/regress/sql/select.sql @@ -234,6 +234,13 @@ select unique1, unique2 from onek2 select unique1, unique2 from onek2 where (unique2 = 11 and stringu1 < 'B') or unique1 = 0; +CREATE TEMP TABLE a (id int PRIMARY KEY); +explain (costs off) +SELECT * FROM a WHERE id IS NULL; + +explain (costs off) +SELECT * FROM a WHERE id IS NOT NULL; + -- -- Test some corner cases that have been known to confuse the planner --