diff options
-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 | 2 | ||||
-rw-r--r-- | src/backend/utils/cache/relcache.c | 5 | ||||
-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 |
8 files changed, 105 insertions, 26 deletions
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 716af49d465..615f1cbc01d 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -742,7 +742,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 e400ea983cc..9c7172c7751 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1696,7 +1696,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 1176e819dd0..7e8b8e7abca 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 f037f90d985..ac57d0f2b89 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -1056,7 +1056,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) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 4ebe103c1ad..afaf8794f00 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -4091,7 +4091,8 @@ 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); @@ -4159,7 +4160,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 46fadff5360..f87bcb6e855 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 prepsecurity.c diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 4f465a5f1ef..50d3403011a 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1591,3 +1591,27 @@ FROM generate_series(1, 3) g(i); 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 diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 0e390cda3e4..385359b7611 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -562,3 +562,15 @@ FROM generate_series(1, 3) g(i); 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; +select * from cnullparent where f1 = 2; +drop table cnullparent cascade; |