diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index fcf7655..786c05d 100644 *** a/src/backend/catalog/partition.c --- b/src/backend/catalog/partition.c *************** get_proposed_default_constraint(List *ne *** 3204,3215 **** defPartConstraint = makeBoolExpr(NOT_EXPR, list_make1(defPartConstraint), -1); defPartConstraint = (Expr *) eval_const_expressions(NULL, (Node *) defPartConstraint); ! defPartConstraint = canonicalize_qual(defPartConstraint); ! return list_make1(defPartConstraint); } /* --- 3204,3217 ---- defPartConstraint = makeBoolExpr(NOT_EXPR, list_make1(defPartConstraint), -1); + + /* Simplify, to put the negated expression into canonical form */ defPartConstraint = (Expr *) eval_const_expressions(NULL, (Node *) defPartConstraint); ! defPartConstraint = canonicalize_qual(defPartConstraint, true); ! return make_ands_implicit(defPartConstraint); } /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e559afb..46a648a 100644 *** a/src/backend/commands/tablecmds.c --- b/src/backend/commands/tablecmds.c *************** PartConstraintImpliedByRelConstraint(Rel *** 13719,13725 **** * fail to detect valid matches without this. */ cexpr = eval_const_expressions(NULL, cexpr); ! cexpr = (Node *) canonicalize_qual((Expr *) cexpr); existConstraint = list_concat(existConstraint, make_ands_implicit((Expr *) cexpr)); --- 13719,13725 ---- * fail to detect valid matches without this. */ cexpr = eval_const_expressions(NULL, cexpr); ! cexpr = (Node *) canonicalize_qual((Expr *) cexpr, true); existConstraint = list_concat(existConstraint, make_ands_implicit((Expr *) cexpr)); *************** ATExecAttachPartition(List **wqueue, Rel *** 14058,14067 **** /* Skip validation if there are no constraints to validate. */ if (partConstraint) { partConstraint = (List *) eval_const_expressions(NULL, (Node *) partConstraint); ! partConstraint = (List *) canonicalize_qual((Expr *) partConstraint); partConstraint = list_make1(make_ands_explicit(partConstraint)); /* --- 14058,14075 ---- /* Skip validation if there are no constraints to validate. */ if (partConstraint) { + /* + * Run the partition quals through const-simplification similar to + * check constraints. We skip canonicalize_qual, though, because + * partition quals should be in canonical form already; also, since + * the qual is in implicit-AND format, we'd have to explicitly convert + * it to explicit-AND format and back again. + */ partConstraint = (List *) eval_const_expressions(NULL, (Node *) partConstraint); ! ! /* XXX this sure looks wrong */ partConstraint = list_make1(make_ands_explicit(partConstraint)); /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 14b7bec..24e6c46 100644 *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *************** preprocess_expression(PlannerInfo *root, *** 988,994 **** */ if (kind == EXPRKIND_QUAL) { ! expr = (Node *) canonicalize_qual((Expr *) expr); #ifdef OPTIMIZER_DEBUG printf("After canonicalize_qual()\n"); --- 988,994 ---- */ if (kind == EXPRKIND_QUAL) { ! expr = (Node *) canonicalize_qual((Expr *) expr, false); #ifdef OPTIMIZER_DEBUG printf("After canonicalize_qual()\n"); diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 46367cb..dc86dd5 100644 *** a/src/backend/optimizer/plan/subselect.c --- b/src/backend/optimizer/plan/subselect.c *************** convert_EXISTS_to_ANY(PlannerInfo *root, *** 1740,1746 **** * subroot. */ whereClause = eval_const_expressions(root, whereClause); ! whereClause = (Node *) canonicalize_qual((Expr *) whereClause); whereClause = (Node *) make_ands_implicit((Expr *) whereClause); /* --- 1740,1746 ---- * subroot. */ whereClause = eval_const_expressions(root, whereClause); ! whereClause = (Node *) canonicalize_qual((Expr *) whereClause, false); whereClause = (Node *) make_ands_implicit((Expr *) whereClause); /* diff --git a/src/backend/optimizer/prep/prepqual.c b/src/backend/optimizer/prep/prepqual.c index cb1f485..52f8893 100644 *** a/src/backend/optimizer/prep/prepqual.c --- b/src/backend/optimizer/prep/prepqual.c *************** *** 39,45 **** static List *pull_ands(List *andlist); static List *pull_ors(List *orlist); ! static Expr *find_duplicate_ors(Expr *qual); static Expr *process_duplicate_ors(List *orlist); --- 39,45 ---- static List *pull_ands(List *andlist); static List *pull_ors(List *orlist); ! static Expr *find_duplicate_ors(Expr *qual, bool is_check); static Expr *process_duplicate_ors(List *orlist); *************** negate_clause(Node *node) *** 269,274 **** --- 269,279 ---- * canonicalize_qual * Convert a qualification expression to the most useful form. * + * This is primarily intended to be used on top-level WHERE (or JOIN/ON) + * clauses. It can also be used on top-level CHECK constraints, for which + * pass is_check = true. DO NOT call it on any expression that is not known + * to be one or the other, as it might apply inappropriate simplifications. + * * The name of this routine is a holdover from a time when it would try to * force the expression into canonical AND-of-ORs or OR-of-ANDs form. * Eventually, we recognized that that had more theoretical purity than *************** negate_clause(Node *node) *** 283,289 **** * Returns the modified qualification. */ Expr * ! canonicalize_qual(Expr *qual) { Expr *newqual; --- 288,294 ---- * Returns the modified qualification. */ Expr * ! canonicalize_qual(Expr *qual, bool is_check) { Expr *newqual; *************** canonicalize_qual(Expr *qual) *** 291,302 **** if (qual == NULL) return NULL; /* * Pull up redundant subclauses in OR-of-AND trees. We do this only * within the top-level AND/OR structure; there's no point in looking * deeper. Also remove any NULL constants in the top-level structure. */ ! newqual = find_duplicate_ors(qual); return newqual; } --- 296,310 ---- if (qual == NULL) return NULL; + /* This should not be invoked on quals in implicit-AND format */ + Assert(!IsA(qual, List)); + /* * Pull up redundant subclauses in OR-of-AND trees. We do this only * within the top-level AND/OR structure; there's no point in looking * deeper. Also remove any NULL constants in the top-level structure. */ ! newqual = find_duplicate_ors(qual, is_check); return newqual; } *************** pull_ors(List *orlist) *** 395,410 **** * Only the top-level AND/OR structure is searched. * * While at it, we remove any NULL constants within the top-level AND/OR ! * structure, eg "x OR NULL::boolean" is reduced to "x". In general that ! * would change the result, so eval_const_expressions can't do it; but at ! * top level of WHERE, we don't need to distinguish between FALSE and NULL ! * results, so it's valid to treat NULL::boolean the same as FALSE and then ! * simplify AND/OR accordingly. * * Returns the modified qualification. AND/OR flatness is preserved. */ static Expr * ! find_duplicate_ors(Expr *qual) { if (or_clause((Node *) qual)) { --- 403,419 ---- * Only the top-level AND/OR structure is searched. * * While at it, we remove any NULL constants within the top-level AND/OR ! * structure, eg in a WHERE clause, "x OR NULL::boolean" is reduced to "x". ! * In general that would change the result, so eval_const_expressions can't ! * do it; but at top level of WHERE, we don't need to distinguish between ! * FALSE and NULL results, so it's valid to treat NULL::boolean the same ! * as FALSE and then simplify AND/OR accordingly. Conversely, in a top-level ! * CHECK constraint, we may treat a NULL the same as TRUE. * * Returns the modified qualification. AND/OR flatness is preserved. */ static Expr * ! find_duplicate_ors(Expr *qual, bool is_check) { if (or_clause((Node *) qual)) { *************** find_duplicate_ors(Expr *qual) *** 416,433 **** { Expr *arg = (Expr *) lfirst(temp); ! arg = find_duplicate_ors(arg); /* Get rid of any constant inputs */ if (arg && IsA(arg, Const)) { Const *carg = (Const *) arg; ! /* Drop constant FALSE or NULL */ ! if (carg->constisnull || !DatumGetBool(carg->constvalue)) ! continue; ! /* constant TRUE, so OR reduces to TRUE */ ! return arg; } orlist = lappend(orlist, arg); --- 425,453 ---- { Expr *arg = (Expr *) lfirst(temp); ! arg = find_duplicate_ors(arg, is_check); /* Get rid of any constant inputs */ if (arg && IsA(arg, Const)) { Const *carg = (Const *) arg; ! if (is_check) ! { ! /* Within OR in CHECK, drop constant FALSE */ ! if (!carg->constisnull && !DatumGetBool(carg->constvalue)) ! continue; ! /* Constant TRUE or NULL, so OR reduces to TRUE */ ! return (Expr *) makeBoolConst(true, false); ! } ! else ! { ! /* Within OR in WHERE, drop constant FALSE or NULL */ ! if (carg->constisnull || !DatumGetBool(carg->constvalue)) ! continue; ! /* Constant TRUE, so OR reduces to TRUE */ ! return arg; ! } } orlist = lappend(orlist, arg); *************** find_duplicate_ors(Expr *qual) *** 449,466 **** { Expr *arg = (Expr *) lfirst(temp); ! arg = find_duplicate_ors(arg); /* Get rid of any constant inputs */ if (arg && IsA(arg, Const)) { Const *carg = (Const *) arg; ! /* Drop constant TRUE */ ! if (!carg->constisnull && DatumGetBool(carg->constvalue)) ! continue; ! /* constant FALSE or NULL, so AND reduces to FALSE */ ! return (Expr *) makeBoolConst(false, false); } andlist = lappend(andlist, arg); --- 469,497 ---- { Expr *arg = (Expr *) lfirst(temp); ! arg = find_duplicate_ors(arg, is_check); /* Get rid of any constant inputs */ if (arg && IsA(arg, Const)) { Const *carg = (Const *) arg; ! if (is_check) ! { ! /* Within AND in CHECK, drop constant TRUE or NULL */ ! if (carg->constisnull || DatumGetBool(carg->constvalue)) ! continue; ! /* Constant FALSE, so AND reduces to FALSE */ ! return arg; ! } ! else ! { ! /* Within AND in WHERE, drop constant TRUE */ ! if (!carg->constisnull && DatumGetBool(carg->constvalue)) ! continue; ! /* Constant FALSE or NULL, so AND reduces to FALSE */ ! return (Expr *) makeBoolConst(false, false); ! } } andlist = lappend(andlist, arg); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 7c4cd8a..bd3a0c4 100644 *** a/src/backend/optimizer/util/plancat.c --- b/src/backend/optimizer/util/plancat.c *************** get_relation_constraints(PlannerInfo *ro *** 1209,1215 **** */ cexpr = eval_const_expressions(root, cexpr); ! cexpr = (Node *) canonicalize_qual((Expr *) cexpr); /* Fix Vars to have the desired varno */ if (varno != 1) --- 1209,1215 ---- */ cexpr = eval_const_expressions(root, cexpr); ! cexpr = (Node *) canonicalize_qual((Expr *) cexpr, true); /* Fix Vars to have the desired varno */ if (varno != 1) *************** get_relation_constraints(PlannerInfo *ro *** 1262,1272 **** if (pcqual) { /* ! * Run each expression through const-simplification and ! * canonicalization similar to check constraints. */ pcqual = (List *) eval_const_expressions(root, (Node *) pcqual); - pcqual = (List *) canonicalize_qual((Expr *) pcqual); /* Fix Vars to have the desired varno */ if (varno != 1) --- 1262,1274 ---- if (pcqual) { /* ! * Run the partition quals through const-simplification similar to ! * check constraints. We skip canonicalize_qual, though, because ! * partition quals should be in canonical form already; also, since ! * the qual is in implicit-AND format, we'd have to explicitly convert ! * it to explicit-AND format and back again. */ pcqual = (List *) eval_const_expressions(root, (Node *) pcqual); /* Fix Vars to have the desired varno */ if (varno != 1) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 9ee78f8..6ab4db2 100644 *** a/src/backend/utils/cache/relcache.c --- b/src/backend/utils/cache/relcache.c *************** RelationBuildPartitionKey(Relation relat *** 900,907 **** * will be comparing them to similarly-processed qual clause operands, * and may fail to detect valid matches without this step; fix * opfuncids while at it. We don't need to bother with ! * canonicalize_qual() though, because partition expressions are not ! * full-fledged qualification clauses. */ expr = eval_const_expressions(NULL, expr); fix_opfuncids(expr); --- 900,908 ---- * will be comparing them to similarly-processed qual clause operands, * and may fail to detect valid matches without this step; fix * opfuncids while at it. We don't need to bother with ! * canonicalize_qual() though, because partition expressions should be ! * in canonical form already (ie, no need for OR-merging or constant ! * elimination). */ expr = eval_const_expressions(NULL, expr); fix_opfuncids(expr); *************** RelationGetIndexExpressions(Relation rel *** 4713,4724 **** * Run the expressions through eval_const_expressions. This is not just an * optimization, but is necessary, because the planner will be comparing * them to similarly-processed qual clauses, and may fail to detect valid ! * matches without this. We don't bother with canonicalize_qual, however. */ result = (List *) eval_const_expressions(NULL, (Node *) result); - result = (List *) canonicalize_qual((Expr *) result); - /* May as well fix opfuncids too */ fix_opfuncids((Node *) result); --- 4714,4724 ---- * Run the expressions through eval_const_expressions. This is not just an * optimization, but is necessary, because the planner will be comparing * them to similarly-processed qual clauses, and may fail to detect valid ! * matches without this. We must not use canonicalize_qual, however, ! * since these aren't qual expressions. */ result = (List *) eval_const_expressions(NULL, (Node *) result); /* May as well fix opfuncids too */ fix_opfuncids((Node *) result); *************** RelationGetIndexPredicate(Relation relat *** 4783,4789 **** */ result = (List *) eval_const_expressions(NULL, (Node *) result); ! result = (List *) canonicalize_qual((Expr *) result); /* Also convert to implicit-AND format */ result = make_ands_implicit((Expr *) result); --- 4783,4789 ---- */ result = (List *) eval_const_expressions(NULL, (Node *) result); ! result = (List *) canonicalize_qual((Expr *) result, false); /* Also convert to implicit-AND format */ result = make_ands_implicit((Expr *) result); diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h index 89b7ef3..3860877 100644 *** a/src/include/optimizer/prep.h --- b/src/include/optimizer/prep.h *************** extern Relids get_relids_for_join(Planne *** 33,39 **** * prototypes for prepqual.c */ extern Node *negate_clause(Node *node); ! extern Expr *canonicalize_qual(Expr *qual); /* * prototypes for preptlist.c --- 33,39 ---- * prototypes for prepqual.c */ extern Node *negate_clause(Node *node); ! extern Expr *canonicalize_qual(Expr *qual, bool is_check); /* * prototypes for preptlist.c diff --git a/src/test/modules/test_predtest/test_predtest.c b/src/test/modules/test_predtest/test_predtest.c index 4a3b14a..51320ad 100644 *** a/src/test/modules/test_predtest/test_predtest.c --- b/src/test/modules/test_predtest/test_predtest.c *************** *** 19,25 **** #include "funcapi.h" #include "optimizer/clauses.h" #include "optimizer/predtest.h" - #include "optimizer/prep.h" #include "utils/builtins.h" PG_MODULE_MAGIC; --- 19,24 ---- *************** test_predtest(PG_FUNCTION_ARGS) *** 137,154 **** /* * Because the clauses are in the SELECT list, preprocess_expression did ! * not pass them through canonicalize_qual nor make_ands_implicit. We can ! * do that here, though, and should do so to match the planner's normal ! * usage of the predicate proof functions. * ! * This still does not exactly duplicate the normal usage of the proof ! * functions, in that they are often given qual clauses containing ! * RestrictInfo nodes. But since predtest.c just looks through those ! * anyway, it seems OK to not worry about that point. */ - clause1 = canonicalize_qual(clause1); - clause2 = canonicalize_qual(clause2); - clause1 = (Expr *) make_ands_implicit(clause1); clause2 = (Expr *) make_ands_implicit(clause2); --- 136,153 ---- /* * Because the clauses are in the SELECT list, preprocess_expression did ! * not pass them through canonicalize_qual nor make_ands_implicit. * ! * We can't do canonicalize_qual here, since it's unclear whether the ! * expressions ought to be treated as WHERE or CHECK clauses. Fortunately, ! * useful test expressions wouldn't be affected by those transformations ! * anyway. We should do make_ands_implicit, though. ! * ! * Another way in which this does not exactly duplicate the normal usage ! * of the proof functions is that they are often given qual clauses ! * containing RestrictInfo nodes. But since predtest.c just looks through ! * those anyway, it seems OK to not worry about that point. */ clause1 = (Expr *) make_ands_implicit(clause1); clause2 = (Expr *) make_ands_implicit(clause2); diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index a79f891..d768dc0 100644 *** a/src/test/regress/expected/inherit.out --- b/src/test/regress/expected/inherit.out *************** reset enable_seqscan; *** 1661,1666 **** --- 1661,1690 ---- reset enable_indexscan; reset enable_bitmapscan; -- + -- Check handling of a constant-null CHECK constraint + -- + create table cnullparent (f1 int); + create table cnullchild (check (f1 = 1 or f1 = null)) inherits(cnullparent); + insert into cnullchild values(1); + insert into cnullchild values(2); + insert into cnullchild values(null); + select * from cnullparent; + f1 + ---- + 1 + 2 + + (3 rows) + + select * from cnullparent where f1 = 2; + f1 + ---- + 2 + (1 row) + + drop table cnullparent cascade; + NOTICE: drop cascades to table cnullchild + -- -- Check that constraint exclusion works correctly with partitions using -- implicit constraints generated from the partition bound information. -- diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 2e42ae1..9397f72 100644 *** a/src/test/regress/sql/inherit.sql --- b/src/test/regress/sql/inherit.sql *************** reset enable_indexscan; *** 612,617 **** --- 612,629 ---- reset enable_bitmapscan; -- + -- Check handling of a constant-null CHECK constraint + -- + create table cnullparent (f1 int); + create table cnullchild (check (f1 = 1 or f1 = null)) inherits(cnullparent); + insert into cnullchild values(1); + insert into cnullchild values(2); + insert into cnullchild values(null); + select * from cnullparent; + select * from cnullparent where f1 = 2; + drop table cnullparent cascade; + + -- -- Check that constraint exclusion works correctly with partitions using -- implicit constraints generated from the partition bound information. --