diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/commands/tablecmds.c | 14 | ||||
-rw-r--r-- | src/backend/optimizer/plan/planner.c | 2 | ||||
-rw-r--r-- | src/backend/optimizer/plan/subselect.c | 2 | ||||
-rw-r--r-- | src/backend/optimizer/prep/prepqual.c | 83 | ||||
-rw-r--r-- | src/backend/optimizer/util/plancat.c | 10 | ||||
-rw-r--r-- | src/backend/utils/cache/relcache.c | 10 | ||||
-rw-r--r-- | src/include/optimizer/prep.h | 1 | ||||
-rw-r--r-- | src/test/regress/expected/inherit.out | 24 | ||||
-rw-r--r-- | src/test/regress/sql/inherit.sql | 12 |
9 files changed, 122 insertions, 36 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 7ea5c4983cd..cb7d8667103 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13636,9 +13636,17 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) partConstraint = list_concat(get_qual_from_partbound(attachrel, rel, cmd->bound), RelationGetPartitionQual(rel)); + + /* + * 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); - partConstraint = (List *) canonicalize_qual((Expr *) partConstraint); + partConstraint = list_make1(make_ands_explicit(partConstraint)); /* @@ -13720,14 +13728,12 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) * to detect valid matches without this. */ cexpr = eval_const_expressions(NULL, cexpr); - cexpr = (Node *) canonicalize_qual((Expr *) cexpr); + cexpr = (Node *) canonicalize_qual_ext((Expr *) cexpr, true); existConstraint = list_concat(existConstraint, make_ands_implicit((Expr *) cexpr)); } - existConstraint = list_make1(make_ands_explicit(existConstraint)); - /* And away we go ... */ if (predicate_implied_by(partConstraint, existConstraint, true)) skip_validate = true; diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 8db98466ede..4dae405bd56 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -941,7 +941,7 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind) */ if (kind == EXPRKIND_QUAL) { - expr = (Node *) canonicalize_qual((Expr *) expr); + expr = (Node *) canonicalize_qual_ext((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 1103984779b..9c792db71e9 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1723,7 +1723,7 @@ convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect, * subroot. */ whereClause = eval_const_expressions(root, whereClause); - whereClause = (Node *) canonicalize_qual((Expr *) whereClause); + whereClause = (Node *) canonicalize_qual_ext((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 f75b3274ad0..0ac04fae9b2 100644 --- a/src/backend/optimizer/prep/prepqual.c +++ b/src/backend/optimizer/prep/prepqual.c @@ -39,7 +39,7 @@ static List *pull_ands(List *andlist); static List *pull_ors(List *orlist); -static Expr *find_duplicate_ors(Expr *qual); +static Expr *find_duplicate_ors(Expr *qual, bool is_check); static Expr *process_duplicate_ors(List *orlist); @@ -269,6 +269,24 @@ negate_clause(Node *node) * canonicalize_qual * Convert a qualification expression to the most useful form. * + * Backwards-compatibility wrapper for use by external code that hasn't + * been updated. + */ +Expr * +canonicalize_qual(Expr *qual) +{ + return canonicalize_qual_ext(qual, false); +} + +/* + * canonicalize_qual_ext + * 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 @@ -283,7 +301,7 @@ negate_clause(Node *node) * Returns the modified qualification. */ Expr * -canonicalize_qual(Expr *qual) +canonicalize_qual_ext(Expr *qual, bool is_check) { Expr *newqual; @@ -296,7 +314,7 @@ canonicalize_qual(Expr *qual) * 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); + newqual = find_duplicate_ors(qual, is_check); return newqual; } @@ -395,16 +413,17 @@ pull_ors(List *orlist) * 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. + * 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) +find_duplicate_ors(Expr *qual, bool is_check) { if (or_clause((Node *) qual)) { @@ -416,18 +435,29 @@ find_duplicate_ors(Expr *qual) { Expr *arg = (Expr *) lfirst(temp); - arg = find_duplicate_ors(arg); + arg = find_duplicate_ors(arg, is_check); /* 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; + 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); @@ -449,18 +479,29 @@ find_duplicate_ors(Expr *qual) { Expr *arg = (Expr *) lfirst(temp); - arg = find_duplicate_ors(arg); + arg = find_duplicate_ors(arg, is_check); /* 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); + 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 dc0b0b07067..646e577272e 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -1187,7 +1187,7 @@ get_relation_constraints(PlannerInfo *root, */ cexpr = eval_const_expressions(root, cexpr); - cexpr = (Node *) canonicalize_qual((Expr *) cexpr); + cexpr = (Node *) canonicalize_qual_ext((Expr *) cexpr, true); /* Fix Vars to have the desired varno */ if (varno != 1) @@ -1240,11 +1240,13 @@ get_relation_constraints(PlannerInfo *root, if (pcqual) { /* - * Run each expression through const-simplification and - * canonicalization similar to check constraints. + * 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); - pcqual = (List *) canonicalize_qual((Expr *) 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 5e497aee9fe..a69b078f91a 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -893,7 +893,8 @@ RelationBuildPartitionKey(Relation relation) * will be comparing them to similarly-processed qual clause operands, * and may fail to detect valid matches without this step. We don't * need to bother with canonicalize_qual() though, because partition - * expressions are not full-fledged qualification clauses. + * expressions should be in canonical form already (ie, no need for + * OR-merging or constant elimination). */ expr = eval_const_expressions(NULL, (Node *) expr); @@ -4742,12 +4743,11 @@ RelationGetIndexExpressions(Relation relation) * 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. + * matches without this. We must not use canonicalize_qual, however, + * since these aren't qual expressions. */ result = (List *) eval_const_expressions(NULL, (Node *) result); - result = (List *) canonicalize_qual((Expr *) result); - /* May as well fix opfuncids too */ fix_opfuncids((Node *) result); @@ -4812,7 +4812,7 @@ RelationGetIndexPredicate(Relation relation) */ result = (List *) eval_const_expressions(NULL, (Node *) result); - result = (List *) canonicalize_qual((Expr *) result); + result = (List *) canonicalize_qual_ext((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 b93faaeb4db..94478e1af8b 100644 --- a/src/include/optimizer/prep.h +++ b/src/include/optimizer/prep.h @@ -34,6 +34,7 @@ extern Relids get_relids_for_join(PlannerInfo *root, int joinrelid); */ extern Node *negate_clause(Node *node); extern Expr *canonicalize_qual(Expr *qual); +extern Expr *canonicalize_qual_ext(Expr *qual, bool is_check); /* * prototypes for preptlist.c diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 164753e418a..2efae8038ff 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1637,6 +1637,30 @@ reset enable_seqscan; 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 eb87ad07755..55770f5681d 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -593,6 +593,18 @@ 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; +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. -- |