extend-predicate-implied-by-v1.patch

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9aef67b..3ca23c8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -65,6 +65,7 @@
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "nodes/parsenodes.h"
+#include "nodes/print.h"
 #include "optimizer/clauses.h"
 #include "optimizer/planner.h"
 #include "optimizer/predtest.h"
@@ -13410,7 +13411,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;
@@ -13597,10 +13597,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)
 		{
@@ -13664,59 +13660,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 607a8f9..07ab339 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 94beeb8..344caf4 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 8f9dd90..1a775b2 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);
diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer/util/predtest.c
index c4a04cf..3a5dc31 100644
--- a/src/backend/optimizer/util/predtest.c
+++ b/src/backend/optimizer/util/predtest.c
@@ -77,7 +77,8 @@ typedef struct PredIterInfoData
 	} while (0)
 
 
-static bool predicate_implied_by_recurse(Node *clause, Node *predicate);
+static bool predicate_implied_by_recurse(Node *clause, Node *predicate,
+							 bool restrictinfo_is_check);
 static bool predicate_refuted_by_recurse(Node *clause, Node *predicate);
 static PredClass predicate_classify(Node *clause, PredIterInfo info);
 static void list_startup_fn(Node *clause, PredIterInfo info);
@@ -90,7 +91,8 @@ 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_implied_by_simple_clause(Expr *predicate, Node *clause,
+								   bool restrictinfo_is_check);
 static bool predicate_refuted_by_simple_clause(Expr *predicate, Node *clause);
 static Node *extract_not_arg(Node *clause);
 static Node *extract_strong_not_arg(Node *clause);
@@ -108,7 +110,10 @@ 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.
+ *	  that the given predicate is true.  If restrictioninfo_is_check is
+ *	  true, assume that the clauses in restrictinfo_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,7 +130,8 @@ 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 *restrictinfo_list,
+					 bool restrictinfo_is_check)
 {
 	Node	   *p,
 			   *r;
@@ -151,7 +157,7 @@ predicate_implied_by(List *predicate_list, List *restrictinfo_list)
 		r = (Node *) restrictinfo_list;
 
 	/* And away we go ... */
-	return predicate_implied_by_recurse(r, p);
+	return predicate_implied_by_recurse(r, p, restrictinfo_is_check);
 }
 
 /*
@@ -248,7 +254,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 restrictinfo_is_check)
 {
 	PredIterInfoData clause_info;
 	PredIterInfoData pred_info;
@@ -275,7 +282,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,
+													  restrictinfo_is_check))
 						{
 							result = false;
 							break;
@@ -294,7 +302,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,
+													  restrictinfo_is_check))
 						{
 							result = true;
 							break;
@@ -311,7 +320,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,
+													  restrictinfo_is_check))
 						{
 							result = true;
 							break;
@@ -328,7 +338,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,
+													  restrictinfo_is_check))
 						{
 							result = true;
 							break;
@@ -355,7 +366,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,
+													  restrictinfo_is_check))
 							{
 								presult = true;
 								break;
@@ -382,7 +394,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,
+													  restrictinfo_is_check))
 						{
 							result = false;
 							break;
@@ -404,7 +417,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,
+													  restrictinfo_is_check))
 						{
 							result = false;
 							break;
@@ -421,7 +435,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,
+													  restrictinfo_is_check))
 						{
 							result = true;
 							break;
@@ -437,7 +452,8 @@ predicate_implied_by_recurse(Node *clause, Node *predicate)
 					 */
 					return
 						predicate_implied_by_simple_clause((Expr *) predicate,
-														   clause);
+														   clause,
+													  restrictinfo_is_check);
 			}
 			break;
 	}
@@ -558,7 +574,7 @@ 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, false))
 						return true;
 
 					/*
@@ -634,7 +650,7 @@ 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, false))
 						return true;
 
 					/*
@@ -712,7 +728,7 @@ 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, false))
 						return true;
 
 					/*
@@ -1022,14 +1038,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 restrictinfo_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 +1054,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 restrictinfo_is_check)
 {
 	/* Allow interrupting long proof attempts */
 	CHECK_FOR_INTERRUPTS();
@@ -1047,7 +1065,7 @@ predicate_implied_by_simple_clause(Expr *predicate, Node *clause)
 		return true;
 
 	/* Next try the IS NOT NULL case */
-	if (predicate && IsA(predicate, NullTest) &&
+	if (predicate && !restrictinfo_is_check && IsA(predicate, NullTest) &&
 		((NullTest *) predicate)->nulltesttype == IS_NOT_NULL)
 	{
 		Expr	   *nonnullarg = ((NullTest *) predicate)->arg;
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 300a8ff..22dabf5 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 658a86c..d6b32e0 100644
--- a/src/include/optimizer/predtest.h
+++ b/src/include/optimizer/predtest.h
@@ -18,7 +18,8 @@
 
 
 extern bool predicate_implied_by(List *predicate_list,
-					 List *restrictinfo_list);
+					 List *restrictinfo_list,
+					 bool restrictinfo_is_check);
 extern bool predicate_refuted_by(List *predicate_list,
 					 List *restrictinfo_list);
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index d4dbe65..13d6a4b 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 001717d..5dd1402 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);