From b08df9cab777427fdafe633ca7b8abf29817aa55 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 14 Jun 2017 13:13:11 -0400 Subject: [PATCH] Teach predtest.c about CHECK clauses to fix partitioning bugs. In a CHECK clause, a null result means true, whereas in a WHERE clause it means false. predtest.c provided different functions depending on which set of semantics applied to the predicate being proved, but had no option to control what a null meant in the clauses provided as axioms. Add one. Use that in the partitioning code when figuring out whether the validation scan on a new partition can be skipped. Rip out the old logic that attempted (not very successfully) to compensate for the absence of the necessary support in predtest.c. Ashutosh Bapat and Robert Haas, reviewed by Amit Langote and incorporating feedback from Tom Lane. Discussion: http://postgr.es/m/CAFjFpReT_kq_uwU_B8aWDxR7jNGE=P0iELycdq5oupi=xSQTOw@mail.gmail.com --- src/backend/commands/tablecmds.c | 60 +-------- src/backend/optimizer/path/indxpath.c | 11 +- src/backend/optimizer/plan/createplan.c | 7 +- src/backend/optimizer/util/plancat.c | 6 +- src/backend/optimizer/util/predtest.c | 152 ++++++++++++++-------- src/backend/utils/adt/selfuncs.c | 4 +- src/include/optimizer/predtest.h | 8 +- src/test/regress/expected/alter_table.out | 6 + src/test/regress/sql/alter_table.sql | 7 +- 9 files changed, 129 insertions(+), 132 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9aef67be44..b4425bc6af 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13410,7 +13410,6 @@ ComputePartitionAttrs(Relation rel, List *partParams, AttrNumber *partattrs, static ObjectAddress ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) { - PartitionKey key = RelationGetPartitionKey(rel); Relation attachRel, catalog; List *childrels; @@ -13596,11 +13595,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) { int num_check = attachRel_constr->num_check; int i; - Bitmapset *not_null_attrs = NULL; - List *part_constr; - ListCell *lc; - bool partition_accepts_null = true; - int partnatts; if (attachRel_constr->has_not_null) { @@ -13630,7 +13624,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) ntest->argisrow = false; ntest->location = -1; existConstraint = lappend(existConstraint, ntest); - not_null_attrs = bms_add_member(not_null_attrs, i); } } } @@ -13664,59 +13657,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) existConstraint = list_make1(make_ands_explicit(existConstraint)); /* And away we go ... */ - if (predicate_implied_by(partConstraint, existConstraint)) + if (predicate_implied_by(partConstraint, existConstraint, true)) skip_validate = true; - - /* - * We choose to err on the safer side, i.e., give up on skipping the - * validation scan, if the partition key column doesn't have the NOT - * NULL constraint and the table is to become a list partition that - * does not accept nulls. In this case, the partition predicate - * (partConstraint) does include an 'key IS NOT NULL' expression, - * however, because of the way predicate_implied_by_simple_clause() is - * designed to handle IS NOT NULL predicates in the absence of a IS - * NOT NULL clause, we cannot rely on just the above proof. - * - * That is not an issue in case of a range partition, because if there - * were no NOT NULL constraint defined on the key columns, an error - * would be thrown before we get here anyway. That is not true, - * however, if any of the partition keys is an expression, which is - * handled below. - */ - part_constr = linitial(partConstraint); - part_constr = make_ands_implicit((Expr *) part_constr); - - /* - * part_constr contains an IS NOT NULL expression, if this is a list - * partition that does not accept nulls (in fact, also if this is a - * range partition and some partition key is an expression, but we - * never skip validation in that case anyway; see below) - */ - foreach(lc, part_constr) - { - Node *expr = lfirst(lc); - - if (IsA(expr, NullTest) && - ((NullTest *) expr)->nulltesttype == IS_NOT_NULL) - { - partition_accepts_null = false; - break; - } - } - - partnatts = get_partition_natts(key); - for (i = 0; i < partnatts; i++) - { - AttrNumber partattno; - - partattno = get_partition_col_attnum(key, i); - - /* If partition key is an expression, must not skip validation */ - if (!partition_accepts_null && - (partattno == 0 || - !bms_is_member(partattno, not_null_attrs))) - skip_validate = false; - } } /* It's safe to skip the validation scan after all */ diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 607a8f97bf..07ab33902b 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1210,10 +1210,10 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel, all_clauses = list_concat(list_copy(clauses), other_clauses); - if (!predicate_implied_by(index->indpred, all_clauses)) + if (!predicate_implied_by(index->indpred, all_clauses, false)) continue; /* can't use it at all */ - if (!predicate_implied_by(index->indpred, other_clauses)) + if (!predicate_implied_by(index->indpred, other_clauses, false)) useful_predicate = true; } } @@ -1519,7 +1519,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths) { Node *np = (Node *) lfirst(l); - if (predicate_implied_by(list_make1(np), qualsofar)) + if (predicate_implied_by(list_make1(np), qualsofar, false)) { redundant = true; break; /* out of inner foreach loop */ @@ -2871,7 +2871,8 @@ check_index_predicates(PlannerInfo *root, RelOptInfo *rel) continue; /* ignore non-partial indexes here */ if (!index->predOK) /* don't repeat work if already proven OK */ - index->predOK = predicate_implied_by(index->indpred, clauselist); + index->predOK = predicate_implied_by(index->indpred, clauselist, + false); /* If rel is an update target, leave indrestrictinfo as set above */ if (is_target_rel) @@ -2886,7 +2887,7 @@ check_index_predicates(PlannerInfo *root, RelOptInfo *rel) /* predicate_implied_by() assumes first arg is immutable */ if (contain_mutable_functions((Node *) rinfo->clause) || !predicate_implied_by(list_make1(rinfo->clause), - index->indpred)) + index->indpred, false)) index->indrestrictinfo = lappend(index->indrestrictinfo, rinfo); } } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 94beeb858d..344caf4532 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2576,7 +2576,7 @@ create_indexscan_plan(PlannerInfo *root, if (is_redundant_derived_clause(rinfo, indexquals)) continue; /* derived from same EquivalenceClass */ if (!contain_mutable_functions((Node *) rinfo->clause) && - predicate_implied_by(list_make1(rinfo->clause), indexquals)) + predicate_implied_by(list_make1(rinfo->clause), indexquals, false)) continue; /* provably implied by indexquals */ qpqual = lappend(qpqual, rinfo); } @@ -2737,7 +2737,7 @@ create_bitmap_scan_plan(PlannerInfo *root, if (rinfo->parent_ec && list_member_ptr(indexECs, rinfo->parent_ec)) continue; /* derived from same EquivalenceClass */ if (!contain_mutable_functions(clause) && - predicate_implied_by(list_make1(clause), indexquals)) + predicate_implied_by(list_make1(clause), indexquals, false)) continue; /* provably implied by indexquals */ qpqual = lappend(qpqual, rinfo); } @@ -2968,7 +2968,8 @@ create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, * the conditions that got pushed into the bitmapqual. Avoid * generating redundant conditions. */ - if (!predicate_implied_by(list_make1(pred), ipath->indexclauses)) + if (!predicate_implied_by(list_make1(pred), ipath->indexclauses, + false)) { *qual = lappend(*qual, pred); *indexqual = lappend(*indexqual, pred); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 8f9dd9099b..939045dc41 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -776,7 +776,7 @@ infer_arbiter_indexes(PlannerInfo *root) */ predExprs = RelationGetIndexPredicate(idxRel); - if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere)) + if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false)) goto next; results = lappend_oid(results, idxForm->indexrelid); @@ -1399,7 +1399,7 @@ relation_excluded_by_constraints(PlannerInfo *root, safe_restrictions = lappend(safe_restrictions, rinfo->clause); } - if (predicate_refuted_by(safe_restrictions, safe_restrictions)) + if (predicate_refuted_by(safe_restrictions, safe_restrictions, false)) return true; /* Only plain relations have constraints */ @@ -1438,7 +1438,7 @@ relation_excluded_by_constraints(PlannerInfo *root, * have volatile and nonvolatile subclauses, and it's OK to make * deductions with the nonvolatile parts. */ - if (predicate_refuted_by(safe_constraints, rel->baserestrictinfo)) + if (predicate_refuted_by(safe_constraints, rel->baserestrictinfo, false)) return true; return false; diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer/util/predtest.c index c4a04cfa95..06fce8458c 100644 --- a/src/backend/optimizer/util/predtest.c +++ b/src/backend/optimizer/util/predtest.c @@ -77,8 +77,10 @@ typedef struct PredIterInfoData } while (0) -static bool predicate_implied_by_recurse(Node *clause, Node *predicate); -static bool predicate_refuted_by_recurse(Node *clause, Node *predicate); +static bool predicate_implied_by_recurse(Node *clause, Node *predicate, + bool clause_is_check); +static bool predicate_refuted_by_recurse(Node *clause, Node *predicate, + bool clause_is_check); static PredClass predicate_classify(Node *clause, PredIterInfo info); static void list_startup_fn(Node *clause, PredIterInfo info); static Node *list_next_fn(PredIterInfo info); @@ -90,8 +92,10 @@ static void arrayconst_cleanup_fn(PredIterInfo info); static void arrayexpr_startup_fn(Node *clause, PredIterInfo info); static Node *arrayexpr_next_fn(PredIterInfo info); static void arrayexpr_cleanup_fn(PredIterInfo info); -static bool predicate_implied_by_simple_clause(Expr *predicate, Node *clause); -static bool predicate_refuted_by_simple_clause(Expr *predicate, Node *clause); +static bool predicate_implied_by_simple_clause(Expr *predicate, Node *clause, + bool clause_is_check); +static bool predicate_refuted_by_simple_clause(Expr *predicate, Node *clause, + bool clause_is_check); static Node *extract_not_arg(Node *clause); static Node *extract_strong_not_arg(Node *clause); static bool list_member_strip(List *list, Expr *datum); @@ -107,8 +111,11 @@ static void InvalidateOprProofCacheCallBack(Datum arg, int cacheid, uint32 hashv /* * predicate_implied_by - * Recursively checks whether the clauses in restrictinfo_list imply - * that the given predicate is true. + * Recursively checks whether the clauses in clause_list imply that the + * given predicate is true. If clause_is_check is true, assume that the + * clauses in clause_list are CHECK constraints (where null is + * effectively true) rather than WHERE clauses (where null is effectively + * false). * * The top-level List structure of each list corresponds to an AND list. * We assume that eval_const_expressions() has been applied and so there @@ -125,14 +132,15 @@ static void InvalidateOprProofCacheCallBack(Datum arg, int cacheid, uint32 hashv * the plan and the time we execute the plan. */ bool -predicate_implied_by(List *predicate_list, List *restrictinfo_list) +predicate_implied_by(List *predicate_list, List *clause_list, + bool clause_is_check) { Node *p, *r; if (predicate_list == NIL) return true; /* no predicate: implication is vacuous */ - if (restrictinfo_list == NIL) + if (clause_list == NIL) return false; /* no restriction: implication must fail */ /* @@ -145,19 +153,22 @@ predicate_implied_by(List *predicate_list, List *restrictinfo_list) p = (Node *) linitial(predicate_list); else p = (Node *) predicate_list; - if (list_length(restrictinfo_list) == 1) - r = (Node *) linitial(restrictinfo_list); + if (list_length(clause_list) == 1) + r = (Node *) linitial(clause_list); else - r = (Node *) restrictinfo_list; + r = (Node *) clause_list; /* And away we go ... */ - return predicate_implied_by_recurse(r, p); + return predicate_implied_by_recurse(r, p, clause_is_check); } /* * predicate_refuted_by - * Recursively checks whether the clauses in restrictinfo_list refute - * the given predicate (that is, prove it false). + * Recursively checks whether the clauses in clause_list refute the given + * predicate (that is, prove it false). If clause_is_check is true, assume + * that the clauses in clause_list are CHECK constraints (where null is + * effectively true) rather than WHERE clauses (where null is effectively + * false). * * This is NOT the same as !(predicate_implied_by), though it is similar * in the technique and structure of the code. @@ -183,14 +194,15 @@ predicate_implied_by(List *predicate_list, List *restrictinfo_list) * time we make the plan and the time we execute the plan. */ bool -predicate_refuted_by(List *predicate_list, List *restrictinfo_list) +predicate_refuted_by(List *predicate_list, List *clause_list, + bool clause_is_check) { Node *p, *r; if (predicate_list == NIL) return false; /* no predicate: no refutation is possible */ - if (restrictinfo_list == NIL) + if (clause_list == NIL) return false; /* no restriction: refutation must fail */ /* @@ -203,13 +215,13 @@ predicate_refuted_by(List *predicate_list, List *restrictinfo_list) p = (Node *) linitial(predicate_list); else p = (Node *) predicate_list; - if (list_length(restrictinfo_list) == 1) - r = (Node *) linitial(restrictinfo_list); + if (list_length(clause_list) == 1) + r = (Node *) linitial(clause_list); else - r = (Node *) restrictinfo_list; + r = (Node *) clause_list; /* And away we go ... */ - return predicate_refuted_by_recurse(r, p); + return predicate_refuted_by_recurse(r, p, clause_is_check); } /*---------- @@ -248,7 +260,8 @@ predicate_refuted_by(List *predicate_list, List *restrictinfo_list) *---------- */ static bool -predicate_implied_by_recurse(Node *clause, Node *predicate) +predicate_implied_by_recurse(Node *clause, Node *predicate, + bool clause_is_check) { PredIterInfoData clause_info; PredIterInfoData pred_info; @@ -275,7 +288,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(pitem, predicate, pred_info) { - if (!predicate_implied_by_recurse(clause, pitem)) + if (!predicate_implied_by_recurse(clause, pitem, + clause_is_check)) { result = false; break; @@ -294,7 +308,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) result = false; iterate_begin(pitem, predicate, pred_info) { - if (predicate_implied_by_recurse(clause, pitem)) + if (predicate_implied_by_recurse(clause, pitem, + clause_is_check)) { result = true; break; @@ -311,7 +326,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) */ iterate_begin(citem, clause, clause_info) { - if (predicate_implied_by_recurse(citem, predicate)) + if (predicate_implied_by_recurse(citem, predicate, + clause_is_check)) { result = true; break; @@ -328,7 +344,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) result = false; iterate_begin(citem, clause, clause_info) { - if (predicate_implied_by_recurse(citem, predicate)) + if (predicate_implied_by_recurse(citem, predicate, + clause_is_check)) { result = true; break; @@ -355,7 +372,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) iterate_begin(pitem, predicate, pred_info) { - if (predicate_implied_by_recurse(citem, pitem)) + if (predicate_implied_by_recurse(citem, pitem, + clause_is_check)) { presult = true; break; @@ -382,7 +400,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(citem, clause, clause_info) { - if (!predicate_implied_by_recurse(citem, predicate)) + if (!predicate_implied_by_recurse(citem, predicate, + clause_is_check)) { result = false; break; @@ -404,7 +423,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(pitem, predicate, pred_info) { - if (!predicate_implied_by_recurse(clause, pitem)) + if (!predicate_implied_by_recurse(clause, pitem, + clause_is_check)) { result = false; break; @@ -421,7 +441,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) result = false; iterate_begin(pitem, predicate, pred_info) { - if (predicate_implied_by_recurse(clause, pitem)) + if (predicate_implied_by_recurse(clause, pitem, + clause_is_check)) { result = true; break; @@ -437,7 +458,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) */ return predicate_implied_by_simple_clause((Expr *) predicate, - clause); + clause, + clause_is_check); } break; } @@ -478,7 +500,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate) *---------- */ static bool -predicate_refuted_by_recurse(Node *clause, Node *predicate) +predicate_refuted_by_recurse(Node *clause, Node *predicate, + bool clause_is_check) { PredIterInfoData clause_info; PredIterInfoData pred_info; @@ -508,7 +531,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = false; iterate_begin(pitem, predicate, pred_info) { - if (predicate_refuted_by_recurse(clause, pitem)) + if (predicate_refuted_by_recurse(clause, pitem, + clause_is_check)) { result = true; break; @@ -525,7 +549,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) */ iterate_begin(citem, clause, clause_info) { - if (predicate_refuted_by_recurse(citem, predicate)) + if (predicate_refuted_by_recurse(citem, predicate, + clause_is_check)) { result = true; break; @@ -542,7 +567,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(pitem, predicate, pred_info) { - if (!predicate_refuted_by_recurse(clause, pitem)) + if (!predicate_refuted_by_recurse(clause, pitem, + clause_is_check)) { result = false; break; @@ -558,7 +584,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) */ not_arg = extract_not_arg(predicate); if (not_arg && - predicate_implied_by_recurse(clause, not_arg)) + predicate_implied_by_recurse(clause, not_arg, + clause_is_check)) return true; /* @@ -567,7 +594,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = false; iterate_begin(citem, clause, clause_info) { - if (predicate_refuted_by_recurse(citem, predicate)) + if (predicate_refuted_by_recurse(citem, predicate, + clause_is_check)) { result = true; break; @@ -589,7 +617,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(pitem, predicate, pred_info) { - if (!predicate_refuted_by_recurse(clause, pitem)) + if (!predicate_refuted_by_recurse(clause, pitem, + clause_is_check)) { result = false; break; @@ -611,7 +640,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) iterate_begin(pitem, predicate, pred_info) { - if (predicate_refuted_by_recurse(citem, pitem)) + if (predicate_refuted_by_recurse(citem, pitem, + clause_is_check)) { presult = true; break; @@ -634,7 +664,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) */ not_arg = extract_not_arg(predicate); if (not_arg && - predicate_implied_by_recurse(clause, not_arg)) + predicate_implied_by_recurse(clause, not_arg, + clause_is_check)) return true; /* @@ -643,7 +674,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(citem, clause, clause_info) { - if (!predicate_refuted_by_recurse(citem, predicate)) + if (!predicate_refuted_by_recurse(citem, predicate, + clause_is_check)) { result = false; break; @@ -679,7 +711,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = false; iterate_begin(pitem, predicate, pred_info) { - if (predicate_refuted_by_recurse(clause, pitem)) + if (predicate_refuted_by_recurse(clause, pitem, + clause_is_check)) { result = true; break; @@ -696,7 +729,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) result = true; iterate_begin(pitem, predicate, pred_info) { - if (!predicate_refuted_by_recurse(clause, pitem)) + if (!predicate_refuted_by_recurse(clause, pitem, + clause_is_check)) { result = false; break; @@ -712,7 +746,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) */ not_arg = extract_not_arg(predicate); if (not_arg && - predicate_implied_by_recurse(clause, not_arg)) + predicate_implied_by_recurse(clause, not_arg, + clause_is_check)) return true; /* @@ -720,7 +755,8 @@ predicate_refuted_by_recurse(Node *clause, Node *predicate) */ return predicate_refuted_by_simple_clause((Expr *) predicate, - clause); + clause, + clause_is_check); } break; } @@ -1022,14 +1058,15 @@ arrayexpr_cleanup_fn(PredIterInfo info) * functions in the expression are immutable, ie dependent only on their input * arguments --- but this was checked for the predicate by the caller.) * - * When the predicate is of the form "foo IS NOT NULL", we can conclude that - * the predicate is implied if the clause is a strict operator or function - * that has "foo" as an input. In this case the clause must yield NULL when - * "foo" is NULL, which we can take as equivalent to FALSE because we know - * we are within an AND/OR subtree of a WHERE clause. (Again, "foo" is - * already known immutable, so the clause will certainly always fail.) - * Also, if the clause is just "foo" (meaning it's a boolean variable), - * the predicate is implied since the clause can't be true if "foo" is NULL. + * When clause_is_check is false, we know we are within an AND/OR + * subtree of a WHERE clause. So, if the predicate is of the form "foo IS + * NOT NULL", we can conclude that the predicate is implied if the clause is + * a strict operator or function that has "foo" as an input. In this case + * the clause must yield NULL when "foo" is NULL, which we can take as + * equivalent to FALSE given the context. (Again, "foo" is already known + * immutable, so the clause will certainly always fail.) Also, if the clause + * is just "foo" (meaning it's a boolean variable), the predicate is implied + * since the clause can't be true if "foo" is NULL. * * Finally, if both clauses are binary operator expressions, we may be able * to prove something using the system's knowledge about operators; those @@ -1037,7 +1074,8 @@ arrayexpr_cleanup_fn(PredIterInfo info) *---------- */ static bool -predicate_implied_by_simple_clause(Expr *predicate, Node *clause) +predicate_implied_by_simple_clause(Expr *predicate, Node *clause, + bool clause_is_check) { /* Allow interrupting long proof attempts */ CHECK_FOR_INTERRUPTS(); @@ -1053,7 +1091,7 @@ predicate_implied_by_simple_clause(Expr *predicate, Node *clause) Expr *nonnullarg = ((NullTest *) predicate)->arg; /* row IS NOT NULL does not act in the simple way we have in mind */ - if (!((NullTest *) predicate)->argisrow) + if (!((NullTest *) predicate)->argisrow && !clause_is_check) { if (is_opclause(clause) && list_member_strip(((OpExpr *) clause)->args, nonnullarg) && @@ -1098,7 +1136,8 @@ predicate_implied_by_simple_clause(Expr *predicate, Node *clause) *---------- */ static bool -predicate_refuted_by_simple_clause(Expr *predicate, Node *clause) +predicate_refuted_by_simple_clause(Expr *predicate, Node *clause, + bool clause_is_check) { /* Allow interrupting long proof attempts */ CHECK_FOR_INTERRUPTS(); @@ -1114,6 +1153,9 @@ predicate_refuted_by_simple_clause(Expr *predicate, Node *clause) { Expr *isnullarg = ((NullTest *) predicate)->arg; + if (clause_is_check) + return false; + /* row IS NULL does not act in the simple way we have in mind */ if (((NullTest *) predicate)->argisrow) return false; diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 300a8ff7e5..22dabf59af 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -6671,7 +6671,7 @@ add_predicate_to_quals(IndexOptInfo *index, List *indexQuals) Node *predQual = (Node *) lfirst(lc); List *oneQual = list_make1(predQual); - if (!predicate_implied_by(oneQual, indexQuals)) + if (!predicate_implied_by(oneQual, indexQuals, false)) predExtraQuals = list_concat(predExtraQuals, oneQual); } /* list_concat avoids modifying the passed-in indexQuals list */ @@ -7556,7 +7556,7 @@ gincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, Node *predQual = (Node *) lfirst(l); List *oneQual = list_make1(predQual); - if (!predicate_implied_by(oneQual, indexQuals)) + if (!predicate_implied_by(oneQual, indexQuals, false)) predExtraQuals = list_concat(predExtraQuals, oneQual); } /* list_concat avoids modifying the passed-in indexQuals list */ diff --git a/src/include/optimizer/predtest.h b/src/include/optimizer/predtest.h index 658a86cc15..748cd35611 100644 --- a/src/include/optimizer/predtest.h +++ b/src/include/optimizer/predtest.h @@ -17,9 +17,9 @@ #include "nodes/primnodes.h" -extern bool predicate_implied_by(List *predicate_list, - List *restrictinfo_list); -extern bool predicate_refuted_by(List *predicate_list, - List *restrictinfo_list); +extern bool predicate_implied_by(List *predicate_list, List *clause_list, + bool clause_is_check); +extern bool predicate_refuted_by(List *predicate_list, List *clause_list, + bool clause_is_check); #endif /* PREDTEST_H */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index d4dbe650a3..13d6a4b747 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3338,6 +3338,12 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5); ERROR: partition constraint is violated by some row -- delete the faulting row and also add a constraint to skip the scan DELETE FROM part_5_a WHERE a NOT IN (3); +ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 5); +ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5); +INFO: partition constraint for table "part_5" is implied by existing constraints +ALTER TABLE list_parted2 DETACH PARTITION part_5; +ALTER TABLE part_5 DROP CONSTRAINT check_a; +-- scan should again be skipped, even though NOT NULL is now a column property ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL; ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5); INFO: partition constraint for table "part_5" is implied by existing constraints diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 001717d91c..5dd1402ea6 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -2169,9 +2169,14 @@ ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5); -- delete the faulting row and also add a constraint to skip the scan DELETE FROM part_5_a WHERE a NOT IN (3); -ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL; +ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 5); ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5); +ALTER TABLE list_parted2 DETACH PARTITION part_5; +ALTER TABLE part_5 DROP CONSTRAINT check_a; +-- scan should again be skipped, even though NOT NULL is now a column property +ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL; +ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5); -- check that the table being attached is not already a partition ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2); -- 2.40.0