v13-0007-Ease-the-restriction-that-a-NOT-ENFORCED-constra.patch

From 3dd65d92b3472b2cd7bbec1841285fc8712534dc Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 7 Feb 2025 09:41:20 +0530
Subject: [PATCH v13 7/9] Ease the restriction that a NOT ENFORCED constraint
 must be INVALID.

In the initial support for NOT ENFORCED check constraints in commit
ca87c415e2fccf81cec6fd45698dde9fae0ab570, we introduced a restriction
that a NOT ENFORCED constraint must be NOT VALID. However, we still
mark a NOT ENFORCED constraint as NOT VALID when adding it to an
already existing table, as validation cannot be performed on a NOT
ENFORCED constraint. Conversely, when the constraint is created along
with the table, it is acceptable to mark the constraint as valid.
---
 src/backend/catalog/heap.c                |  3 +-
 src/backend/catalog/pg_constraint.c       |  2 -
 src/backend/commands/tablecmds.c          | 19 ++---
 src/backend/optimizer/util/plancat.c      |  9 +--
 src/backend/parser/gram.y                 |  9 ---
 src/backend/parser/parse_utilcmd.c        | 13 ++--
 src/backend/utils/adt/ruleutils.c         |  6 +-
 src/test/regress/expected/alter_table.out |  4 +-
 src/test/regress/expected/inherit.out     | 88 ++++++++++++++---------
 src/test/regress/sql/alter_table.sql      |  3 +-
 src/test/regress/sql/inherit.sql          | 13 +++-
 11 files changed, 84 insertions(+), 85 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 956f196fc95..913e63fe93f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2740,7 +2740,7 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
 		 * If the child constraint is "not valid" then cannot merge with a
 		 * valid parent constraint.
 		 */
-		if (is_initially_valid && con->conenforced && !con->convalidated)
+		if (is_initially_valid && !con->convalidated)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
 					 errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"",
@@ -2803,7 +2803,6 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
 		{
 			Assert(is_local);
 			con->conenforced = true;
-			con->convalidated = true;
 		}
 
 		CatalogTupleUpdate(conDesc, &tup->t_self, tup);
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index ac80652baf2..60bc54a96b6 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -102,8 +102,6 @@ CreateConstraintEntry(const char *constraintName,
 
 	/* Only CHECK constraint can be not enforced */
 	Assert(isEnforced || constraintType == CONSTRAINT_CHECK);
-	/* NOT ENFORCED constraint must be NOT VALID */
-	Assert(isEnforced || !isValidated);
 
 	conDesc = table_open(ConstraintRelationId, RowExclusiveLock);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9f26088a0bb..820a837e18b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3166,10 +3166,7 @@ MergeCheckConstraint(List *constraints, const char *name, Node *expr, bool is_en
 			 * marked as ENFORCED because one of the parents is ENFORCED.
 			 */
 			if (!ccon->is_enforced && is_enforced)
-			{
 				ccon->is_enforced = true;
-				ccon->skip_validation = false;
-			}
 
 			return constraints;
 		}
@@ -3190,7 +3187,6 @@ MergeCheckConstraint(List *constraints, const char *name, Node *expr, bool is_en
 	newcon->expr = expr;
 	newcon->inhcount = 1;
 	newcon->is_enforced = is_enforced;
-	newcon->skip_validation = !is_enforced;
 	return lappend(constraints, newcon);
 }
 
@@ -16897,8 +16893,7 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
 			 * If the child constraint is "not valid" then cannot merge with a
 			 * valid parent constraint
 			 */
-			if (parent_con->convalidated && child_con->conenforced &&
-				!child_con->convalidated)
+			if (parent_con->convalidated && !child_con->convalidated)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
 						 errmsg("constraint \"%s\" conflicts with NOT VALID constraint on child table \"%s\"",
@@ -19266,18 +19261,12 @@ ConstraintImpliedByRelConstraint(Relation scanrel, List *testConstraint, List *p
 		Node	   *cexpr;
 
 		/*
-		 * If this constraint hasn't been fully validated yet, we must ignore
-		 * it here.
+		 * If this constraint hasn't been fully validated yet or is not
+		 * enforced, we must ignore it here.
 		 */
-		if (!constr->check[i].ccvalid)
+		if (!constr->check[i].ccvalid || !constr->check[i].ccenforced)
 			continue;
 
-		/*
-		 * NOT ENFORCED constraints are always marked as invalid, which should
-		 * have been ignored.
-		 */
-		Assert(constr->check[i].ccenforced);
-
 		cexpr = stringToNode(constr->check[i].ccbin);
 
 		/*
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 71abb01f655..233a5d8da98 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1302,22 +1302,15 @@ get_relation_constraints(PlannerInfo *root,
 			 * ignore it here.  Also ignore if NO INHERIT and we weren't told
 			 * that that's safe.
 			 */
-			if (!constr->check[i].ccvalid)
+			if (!constr->check[i].ccvalid || !constr->check[i].ccenforced)
 				continue;
 
-			/*
-			 * NOT ENFORCED constraints are always marked as invalid, which
-			 * should have been ignored.
-			 */
-			Assert(constr->check[i].ccenforced);
-
 			/*
 			 * Also ignore if NO INHERIT and we weren't told that that's safe.
 			 */
 			if (constr->check[i].ccnoinherit && !include_noinherit)
 				continue;
 
-
 			cexpr = stringToNode(constr->check[i].ccbin);
 
 			/*
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4835c8dd307..a36e322c78e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -19558,15 +19558,6 @@ processCASbits(int cas_bits, int location, const char *constrType,
 					 errmsg("%s constraints cannot be marked NOT ENFORCED",
 							constrType),
 					 parser_errposition(location)));
-
-		/*
-		 * NB: The validated status is irrelevant when the constraint is set to
-		 * NOT ENFORCED, but for consistency, it should be set accordingly.
-		 * This ensures that if the constraint is later changed to ENFORCED, it
-		 * will automatically be in the correct NOT VALIDATED state.
-		 */
-		if (not_valid)
-			*not_valid = true;
 	}
 
 	if (cas_bits & CAS_ENFORCED)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index eb7716cd84c..9053c898e06 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1452,7 +1452,6 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
 			char	   *ccname = constr->check[ccnum].ccname;
 			char	   *ccbin = constr->check[ccnum].ccbin;
 			bool		ccenforced = constr->check[ccnum].ccenforced;
-			bool		ccvalid = constr->check[ccnum].ccvalid;
 			bool		ccnoinherit = constr->check[ccnum].ccnoinherit;
 			Node	   *ccbin_node;
 			bool		found_whole_row;
@@ -1483,13 +1482,13 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
 			n->conname = pstrdup(ccname);
 			n->location = -1;
 			n->is_enforced = ccenforced;
-			n->initially_valid = ccvalid;
 			n->is_no_inherit = ccnoinherit;
 			n->raw_expr = NULL;
 			n->cooked_expr = nodeToString(ccbin_node);
 
 			/* We can skip validation, since the new table should be empty. */
 			n->skip_validation = true;
+			n->initially_valid = true;
 
 			atsubcmd = makeNode(AlterTableCmd);
 			atsubcmd->subtype = AT_AddConstraint;
@@ -2942,11 +2941,9 @@ transformCheckConstraints(CreateStmtContext *cxt, bool skipValidation)
 		return;
 
 	/*
-	 * When creating a new table (but not a foreign table), we can safely skip
-	 * the validation of check constraints and mark them as valid based on the
-	 * constraint enforcement flag, since NOT ENFORCED constraints must always
-	 * be marked as NOT VALID. (This will override any user-supplied NOT VALID
-	 * flag.)
+	 * If creating a new table (but not a foreign table), we can safely skip
+	 * validation of check constraints, and nonetheless mark them valid. (This
+	 * will override any user-supplied NOT VALID flag.)
 	 */
 	if (skipValidation)
 	{
@@ -2955,7 +2952,7 @@ transformCheckConstraints(CreateStmtContext *cxt, bool skipValidation)
 			Constraint *constraint = (Constraint *) lfirst(ckclist);
 
 			constraint->skip_validation = true;
-			constraint->initially_valid = constraint->is_enforced;
+			constraint->initially_valid = true;
 		}
 	}
 }
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 54dad975553..41751181f61 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2594,12 +2594,10 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
 		appendStringInfoString(&buf, " DEFERRABLE");
 	if (conForm->condeferred)
 		appendStringInfoString(&buf, " INITIALLY DEFERRED");
-
-	/* Validated status is irrelevant when the constraint is NOT ENFORCED. */
+	if (!conForm->convalidated)
+		appendStringInfoString(&buf, " NOT VALID");
 	if (!conForm->conenforced)
 		appendStringInfoString(&buf, " NOT ENFORCED");
-	else if (!conForm->convalidated)
-		appendStringInfoString(&buf, " NOT VALID");
 
 	/* Cleanup */
 	systable_endscan(scandesc);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 362f38856d2..54bdb3f2250 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -507,7 +507,9 @@ ALTER TABLE attmp3 validate constraint attmpconstr;
 ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10); -- fail
 ERROR:  check constraint "b_greater_than_ten" of relation "attmp3" is violated by some row
 ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10) NOT VALID; -- succeeds
-ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK (b > 10) NOT ENFORCED; -- succeeds
+ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK (b > 10) NOT ENFORCED; -- fail
+ERROR:  check constraint "b_greater_than_ten_not_enforced" of relation "attmp3" is violated by some row
+ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK (b > 10) NOT VALID NOT ENFORCED; -- succeeds
 ALTER TABLE attmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- fails
 ERROR:  check constraint "b_greater_than_ten" of relation "attmp3" is violated by some row
 DELETE FROM attmp3 WHERE NOT b > 10;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index dbf3835cb14..d29f870feea 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1332,6 +1332,18 @@ NOTICE:  merging constraint "inh_check_constraint5" with inherited definition
 alter table p1 add constraint inh_check_constraint6 check (f1 < 10) not enforced;
 alter table p1_c1 add constraint inh_check_constraint6 check (f1 < 10) enforced;
 NOTICE:  merging constraint "inh_check_constraint6" with inherited definition
+-- however, cannot merge a not valid enforced child constraint with a valid
+-- not-enforced parent constraint.
+alter table p1_c1 add constraint inh_check_constraint9 check (f1 < 10) not valid enforced;
+alter table p1 add constraint inh_check_constraint9 check (f1 < 10) not enforced;
+ERROR:  constraint "inh_check_constraint9" conflicts with NOT VALID constraint on relation "p1_c1"
+-- but, allowed if the parent constraint is also invalid.
+alter table p1 add constraint inh_check_constraint9 check (f1 < 10) not valid not enforced;
+NOTICE:  merging constraint "inh_check_constraint9" with inherited definition
+-- the invalid state of the child constraint will be ignored here.
+alter table p1 add constraint inh_check_constraint10 check (f1 < 10) not enforced;
+alter table p1_c1 add constraint inh_check_constraint10 check (f1 < 10) not valid enforced;
+NOTICE:  merging constraint "inh_check_constraint10" with inherited definition
 create table p1_c2(f1 int constraint inh_check_constraint4 check (f1 < 10)) inherits(p1);
 NOTICE:  merging column "f1" with inherited definition
 NOTICE:  merging constraint "inh_check_constraint4" with inherited definition
@@ -1353,42 +1365,50 @@ create table p1_fail(f1 int constraint inh_check_constraint6 check (f1 < 10) not
 NOTICE:  merging multiple inherited definitions of column "f1"
 NOTICE:  merging column "f1" with inherited definition
 ERROR:  constraint "inh_check_constraint6" conflicts with NOT ENFORCED constraint on relation "p1_fail"
-select conrelid::regclass::text as relname, conname, conislocal, coninhcount, conenforced
+select conrelid::regclass::text as relname, conname, conislocal, coninhcount, conenforced, convalidated
 from pg_constraint where conname like 'inh\_check\_constraint%'
 order by 1, 2;
- relname |        conname        | conislocal | coninhcount | conenforced 
----------+-----------------------+------------+-------------+-------------
- p1      | inh_check_constraint1 | t          |           0 | t
- p1      | inh_check_constraint2 | t          |           0 | t
- p1      | inh_check_constraint3 | t          |           0 | f
- p1      | inh_check_constraint4 | t          |           0 | f
- p1      | inh_check_constraint5 | t          |           0 | f
- p1      | inh_check_constraint6 | t          |           0 | f
- p1      | inh_check_constraint8 | t          |           0 | t
- p1_c1   | inh_check_constraint1 | t          |           1 | t
- p1_c1   | inh_check_constraint2 | t          |           1 | t
- p1_c1   | inh_check_constraint3 | t          |           1 | f
- p1_c1   | inh_check_constraint4 | t          |           1 | f
- p1_c1   | inh_check_constraint5 | t          |           1 | t
- p1_c1   | inh_check_constraint6 | t          |           1 | t
- p1_c1   | inh_check_constraint7 | t          |           0 | f
- p1_c1   | inh_check_constraint8 | f          |           1 | t
- p1_c2   | inh_check_constraint1 | f          |           1 | t
- p1_c2   | inh_check_constraint2 | f          |           1 | t
- p1_c2   | inh_check_constraint3 | f          |           1 | f
- p1_c2   | inh_check_constraint4 | t          |           1 | t
- p1_c2   | inh_check_constraint5 | f          |           1 | f
- p1_c2   | inh_check_constraint6 | f          |           1 | f
- p1_c2   | inh_check_constraint8 | f          |           1 | t
- p1_c3   | inh_check_constraint1 | f          |           2 | t
- p1_c3   | inh_check_constraint2 | f          |           2 | t
- p1_c3   | inh_check_constraint3 | f          |           2 | f
- p1_c3   | inh_check_constraint4 | f          |           2 | f
- p1_c3   | inh_check_constraint5 | f          |           2 | t
- p1_c3   | inh_check_constraint6 | f          |           2 | t
- p1_c3   | inh_check_constraint7 | f          |           1 | f
- p1_c3   | inh_check_constraint8 | f          |           2 | t
-(30 rows)
+ relname |        conname         | conislocal | coninhcount | conenforced | convalidated 
+---------+------------------------+------------+-------------+-------------+--------------
+ p1      | inh_check_constraint1  | t          |           0 | t           | t
+ p1      | inh_check_constraint10 | t          |           0 | f           | t
+ p1      | inh_check_constraint2  | t          |           0 | t           | t
+ p1      | inh_check_constraint3  | t          |           0 | f           | t
+ p1      | inh_check_constraint4  | t          |           0 | f           | t
+ p1      | inh_check_constraint5  | t          |           0 | f           | t
+ p1      | inh_check_constraint6  | t          |           0 | f           | t
+ p1      | inh_check_constraint8  | t          |           0 | t           | t
+ p1      | inh_check_constraint9  | t          |           0 | f           | f
+ p1_c1   | inh_check_constraint1  | t          |           1 | t           | t
+ p1_c1   | inh_check_constraint10 | t          |           1 | t           | t
+ p1_c1   | inh_check_constraint2  | t          |           1 | t           | t
+ p1_c1   | inh_check_constraint3  | t          |           1 | f           | t
+ p1_c1   | inh_check_constraint4  | t          |           1 | f           | t
+ p1_c1   | inh_check_constraint5  | t          |           1 | t           | t
+ p1_c1   | inh_check_constraint6  | t          |           1 | t           | t
+ p1_c1   | inh_check_constraint7  | t          |           0 | f           | t
+ p1_c1   | inh_check_constraint8  | f          |           1 | t           | t
+ p1_c1   | inh_check_constraint9  | t          |           1 | t           | f
+ p1_c2   | inh_check_constraint1  | f          |           1 | t           | t
+ p1_c2   | inh_check_constraint10 | f          |           1 | f           | t
+ p1_c2   | inh_check_constraint2  | f          |           1 | t           | t
+ p1_c2   | inh_check_constraint3  | f          |           1 | f           | t
+ p1_c2   | inh_check_constraint4  | t          |           1 | t           | t
+ p1_c2   | inh_check_constraint5  | f          |           1 | f           | t
+ p1_c2   | inh_check_constraint6  | f          |           1 | f           | t
+ p1_c2   | inh_check_constraint8  | f          |           1 | t           | t
+ p1_c2   | inh_check_constraint9  | f          |           1 | f           | t
+ p1_c3   | inh_check_constraint1  | f          |           2 | t           | t
+ p1_c3   | inh_check_constraint10 | f          |           2 | t           | t
+ p1_c3   | inh_check_constraint2  | f          |           2 | t           | t
+ p1_c3   | inh_check_constraint3  | f          |           2 | f           | t
+ p1_c3   | inh_check_constraint4  | f          |           2 | f           | t
+ p1_c3   | inh_check_constraint5  | f          |           2 | t           | t
+ p1_c3   | inh_check_constraint6  | f          |           2 | t           | t
+ p1_c3   | inh_check_constraint7  | f          |           1 | f           | t
+ p1_c3   | inh_check_constraint8  | f          |           2 | t           | t
+ p1_c3   | inh_check_constraint9  | f          |           2 | t           | t
+(38 rows)
 
 drop table p1 cascade;
 NOTICE:  drop cascades to 3 other objects
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 84e93ef575e..a9268de14b5 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -387,7 +387,8 @@ ALTER TABLE attmp3 validate constraint attmpconstr;
 -- Try a non-verified CHECK constraint
 ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10); -- fail
 ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten CHECK (b > 10) NOT VALID; -- succeeds
-ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK (b > 10) NOT ENFORCED; -- succeeds
+ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK (b > 10) NOT ENFORCED; -- fail
+ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK (b > 10) NOT VALID NOT ENFORCED; -- succeeds
 ALTER TABLE attmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- fails
 DELETE FROM attmp3 WHERE NOT b > 10;
 ALTER TABLE attmp3 VALIDATE CONSTRAINT b_greater_than_ten; -- succeeds
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 49aae426f3c..35a85203286 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -481,6 +481,17 @@ alter table p1 add constraint inh_check_constraint5 check (f1 < 10) not enforced
 alter table p1 add constraint inh_check_constraint6 check (f1 < 10) not enforced;
 alter table p1_c1 add constraint inh_check_constraint6 check (f1 < 10) enforced;
 
+-- however, cannot merge a not valid enforced child constraint with a valid
+-- not-enforced parent constraint.
+alter table p1_c1 add constraint inh_check_constraint9 check (f1 < 10) not valid enforced;
+alter table p1 add constraint inh_check_constraint9 check (f1 < 10) not enforced;
+-- but, allowed if the parent constraint is also invalid.
+alter table p1 add constraint inh_check_constraint9 check (f1 < 10) not valid not enforced;
+
+-- the invalid state of the child constraint will be ignored here.
+alter table p1 add constraint inh_check_constraint10 check (f1 < 10) not enforced;
+alter table p1_c1 add constraint inh_check_constraint10 check (f1 < 10) not valid enforced;
+
 create table p1_c2(f1 int constraint inh_check_constraint4 check (f1 < 10)) inherits(p1);
 
 -- but reverse is not allowed
@@ -498,7 +509,7 @@ create table p1_c3() inherits(p1, p1_c1);
 -- but not allowed if the child constraint is explicitly asked to be NOT ENFORCED
 create table p1_fail(f1 int constraint inh_check_constraint6 check (f1 < 10) not enforced) inherits(p1, p1_c1);
 
-select conrelid::regclass::text as relname, conname, conislocal, coninhcount, conenforced
+select conrelid::regclass::text as relname, conname, conislocal, coninhcount, conenforced, convalidated
 from pg_constraint where conname like 'inh\_check\_constraint%'
 order by 1, 2;
 
-- 
2.43.5