v2-0001-ALTER-DOMAIN-ADD-NOT-NULL-NOT-VALID.patch

From a840278f579b57cab4c13b91abfcc4f44a6d8a83 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 28 May 2025 15:32:03 +0800
Subject: [PATCH v2 1/1] ALTER DOMAIN ADD NOT NULL NOT VALID

we have NOT NULL NO VALID for table constraints, we can make domain have NOT
VALID not-null constraint too.

ALTER DOMAIN SET NOT NULL works similarly to its ALTER TABLE counterpart.  It
validates an existing invalid NOT NULL constraint and updates
pg_constraint.convalidated to true.

ALTER DOMAIN ADD NOT NULL will add a new, valid not-null constraint.  If the
domain already has an NOT VALID not-null constraint, the command will result in
an error.
If domain already have VALID not-null, add a NOT VALID is no-op.

\dD changes:
for domain's not null not valid constraint: now column "Nullable" will display as
"not null not valid".
domain valid not-null constraint works as before.

discussed: https://postgr.es/m/CACJufxGcABLgmH951SJkkihK+FW8KR3=odBhXEVCF9atQbur2Q@mail.gmail.com
---
 doc/src/sgml/catalogs.sgml           |   1 +
 doc/src/sgml/ref/alter_domain.sgml   |   1 -
 src/backend/catalog/pg_constraint.c  |  10 +--
 src/backend/commands/typecmds.c      | 116 +++++++++++++++++++++++++--
 src/backend/parser/gram.y            |   8 +-
 src/bin/pg_dump/pg_dump.c            |  27 +++++--
 src/bin/pg_dump/t/002_pg_dump.pl     |  16 ++++
 src/bin/psql/describe.c              |   4 +-
 src/test/regress/expected/domain.out |  41 ++++++++++
 src/test/regress/sql/domain.sql      |  22 +++++
 10 files changed, 219 insertions(+), 27 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index cbd4e40a320..fd1b4b0a563 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -9515,6 +9515,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       <para>
        <structfield>typnotnull</structfield> represents a not-null
        constraint on a type.  Used for domains only.
+       Note: the not-null constraint on domain may not validated.
       </para></entry>
      </row>
 
diff --git a/doc/src/sgml/ref/alter_domain.sgml b/doc/src/sgml/ref/alter_domain.sgml
index 74855172222..4807116eb05 100644
--- a/doc/src/sgml/ref/alter_domain.sgml
+++ b/doc/src/sgml/ref/alter_domain.sgml
@@ -92,7 +92,6 @@ ALTER DOMAIN <replaceable class="parameter">name</replaceable>
       valid using <command>ALTER DOMAIN ... VALIDATE CONSTRAINT</command>.
       Newly inserted or updated rows are always checked against all
       constraints, even those marked <literal>NOT VALID</literal>.
-      <literal>NOT VALID</literal> is only accepted for <literal>CHECK</literal> constraints.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 2d5ac1ea813..27d2fee52d3 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -651,8 +651,8 @@ findNotNullConstraint(Oid relid, const char *colname)
 }
 
 /*
- * Find and return the pg_constraint tuple that implements a validated
- * not-null constraint for the given domain.
+ * Find and return the pg_constraint tuple that implements not-null constraint
+ * for the given domain. it may be NOT VALID.
  */
 HeapTuple
 findDomainNotNullConstraint(Oid typid)
@@ -675,13 +675,9 @@ findDomainNotNullConstraint(Oid typid)
 	{
 		Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(conTup);
 
-		/*
-		 * We're looking for a NOTNULL constraint that's marked validated.
-		 */
+		/* We're looking for a NOTNULL constraint */
 		if (con->contype != CONSTRAINT_NOTNULL)
 			continue;
-		if (!con->convalidated)
-			continue;
 
 		/* Found it */
 		retval = heap_copytuple(conTup);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 45ae7472ab5..12744e906ec 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2757,12 +2757,70 @@ AlterDomainNotNull(List *names, bool notNull)
 	checkDomainOwner(tup);
 
 	/* Is the domain already set to the desired constraint? */
-	if (typTup->typnotnull == notNull)
+	if (!typTup->typnotnull && !notNull)
 	{
 		table_close(typrel, RowExclusiveLock);
 		return address;
 	}
 
+	if (typTup->typnotnull && notNull)
+	{
+		ScanKeyData key[1];
+		SysScanDesc scan;
+		Relation	pg_constraint;
+		Form_pg_constraint copy_con;
+		HeapTuple	conTup;
+		HeapTuple	copyTuple;
+
+		pg_constraint = table_open(ConstraintRelationId, AccessShareLock);
+
+		/* Look for NOT NULL Constraints on this domain */
+		ScanKeyInit(&key[0],
+					Anum_pg_constraint_contypid,
+					BTEqualStrategyNumber, F_OIDEQ,
+					ObjectIdGetDatum(domainoid));
+
+		scan = systable_beginscan(pg_constraint, ConstraintTypidIndexId, true,
+								  NULL, 1, key);
+
+		while (HeapTupleIsValid(conTup = systable_getnext(scan)))
+		{
+			Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(conTup);
+
+			if (con->contype != CONSTRAINT_NOTNULL)
+				continue;
+
+			/*
+			 * ALTER DOMAIN SET NOT NULL will validate the NOT VALID not-null
+			 * constraint, also set the pg_constraint.convalidated to true.
+			*/
+			if (!con->convalidated)
+			{
+				validateDomainNotNullConstraint(domainoid);
+				copyTuple = heap_copytuple(conTup);
+
+				copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
+				copy_con->convalidated = true;
+				CatalogTupleUpdate(pg_constraint, &copyTuple->t_self, copyTuple);
+
+				InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
+
+				heap_freetuple(copyTuple);
+			}
+			break;
+		}
+		systable_endscan(scan);
+		table_close(pg_constraint, AccessShareLock);
+
+		table_close(typrel, RowExclusiveLock);
+
+		/*
+		 * If we already validated the existing NOT VALID not-null constraint
+		 * then we don't need install another not-null constraint, exit now.
+		*/
+		return address;
+	}
+
 	if (notNull)
 	{
 		Constraint *constr;
@@ -2990,7 +3048,45 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
 	}
 	else if (constr->contype == CONSTR_NOTNULL)
 	{
-		/* Is the domain already set NOT NULL? */
+		HeapTuple	conTup;
+		ScanKeyData key[1];
+		SysScanDesc scan;
+		Relation	pg_constraint;
+
+		pg_constraint = table_open(ConstraintRelationId, AccessShareLock);
+
+		/* Look for NOT NULL Constraints on this domain */
+		ScanKeyInit(&key[0],
+					Anum_pg_constraint_contypid,
+					BTEqualStrategyNumber, F_OIDEQ,
+					ObjectIdGetDatum(domainoid));
+
+		scan = systable_beginscan(pg_constraint, ConstraintTypidIndexId, true,
+								  NULL, 1, key);
+
+		while (HeapTupleIsValid(conTup = systable_getnext(scan)))
+		{
+			Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(conTup);
+
+			if (con->contype != CONSTRAINT_NOTNULL)
+				continue;
+
+			/*
+			 * can not add not-null constraint if the domain already have NOT
+			 * VALID not-null constraint
+			*/
+			if (!constr->skip_validation && !con->convalidated)
+				ereport(ERROR,
+						errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						errmsg("incompatible NOT VALID constraint \"%s\" on domain \"%s\"",
+								NameStr(con->conname), NameStr(typTup->typname)),
+						errhint("You might need to validate it using %s.",
+								"ALTER DOMAIN ... VALIDATE CONSTRAINT"));
+			break;
+		}
+		systable_endscan(scan);
+		table_close(pg_constraint, AccessShareLock);
+
 		if (typTup->typnotnull)
 		{
 			table_close(typrel, RowExclusiveLock);
@@ -3081,16 +3177,20 @@ AlterDomainValidateConstraint(List *names, const char *constrName)
 						constrName, TypeNameToString(typename))));
 
 	con = (Form_pg_constraint) GETSTRUCT(tuple);
-	if (con->contype != CONSTRAINT_CHECK)
+	if (con->contype != CONSTRAINT_CHECK && con->contype != CONSTRAINT_NOTNULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("constraint \"%s\" of domain \"%s\" is not a check constraint",
+				 errmsg("constraint \"%s\" of domain \"%s\" is not a check or not-null constraint",
 						constrName, TypeNameToString(typename))));
 
-	val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin);
-	conbin = TextDatumGetCString(val);
-
-	validateDomainCheckConstraint(domainoid, conbin);
+	if (con->contype == CONSTRAINT_CHECK)
+	{
+		val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin);
+		conbin = TextDatumGetCString(val);
+		validateDomainCheckConstraint(domainoid, conbin);
+	}
+	else
+		validateDomainNotNullConstraint(domainoid);
 
 	/*
 	 * Now update the catalog, while we have the door open.
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0b5652071d1..8625ff82147 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4391,11 +4391,13 @@ DomainConstraintElem:
 					n->contype = CONSTR_NOTNULL;
 					n->location = @1;
 					n->keys = list_make1(makeString("value"));
-					/* no NOT VALID, NO INHERIT support */
+					/* NO INHERIT is not supported */
 					processCASbits($3, @3, "NOT NULL",
 								   NULL, NULL, NULL,
-								   NULL, NULL, yyscanner);
-					n->initially_valid = true;
+								   &n->skip_validation,
+								   NULL, yyscanner);
+					n->is_enforced = true;
+					n->initially_valid = !n->skip_validation;
 					$$ = (Node *) n;
 				}
 		;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 37432e66efd..88e825f24fe 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8250,7 +8250,8 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
 	int			i_tableoid,
 				i_oid,
 				i_conname,
-				i_consrc;
+				i_consrc,
+				i_contype;
 	int			ntups;
 
 	if (!fout->is_prepared[PREPQUERY_GETDOMAINCONSTRAINTS])
@@ -8260,9 +8261,10 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
 							 "PREPARE getDomainConstraints(pg_catalog.oid) AS\n"
 							 "SELECT tableoid, oid, conname, "
 							 "pg_catalog.pg_get_constraintdef(oid) AS consrc, "
-							 "convalidated "
+							 "convalidated, "
+							 "contype "
 							 "FROM pg_catalog.pg_constraint "
-							 "WHERE contypid = $1 AND contype = 'c' "
+							 "WHERE contypid = $1 "
 							 "ORDER BY conname");
 
 		ExecuteSqlStatement(fout, query->data);
@@ -8282,6 +8284,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
 	i_oid = PQfnumber(res, "oid");
 	i_conname = PQfnumber(res, "conname");
 	i_consrc = PQfnumber(res, "consrc");
+	i_contype = PQfnumber(res, "contype");
 
 	constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
 
@@ -8300,7 +8303,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
 		constrinfo[i].dobj.namespace = tyinfo->dobj.namespace;
 		constrinfo[i].contable = NULL;
 		constrinfo[i].condomain = tyinfo;
-		constrinfo[i].contype = 'c';
+		constrinfo[i].contype = *(PQgetvalue(res, i, i_contype));
 		constrinfo[i].condef = pg_strdup(PQgetvalue(res, i, i_consrc));
 		constrinfo[i].confrelid = InvalidOid;
 		constrinfo[i].conindex = 0;
@@ -12497,8 +12500,18 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
 			appendPQExpBuffer(q, " COLLATE %s", fmtQualifiedDumpable(coll));
 	}
 
-	if (typnotnull[0] == 't')
+	if (typnotnull[0] == 't' && fout->remoteVersion < 190000)
 		appendPQExpBufferStr(q, " NOT NULL");
+	else
+	{
+		for (i = 0; i < tyinfo->nDomChecks; i++)
+		{
+			ConstraintInfo *domcheck = &(tyinfo->domChecks[i]);
+
+			if (!domcheck->separate && domcheck->contype == 'n')
+				appendPQExpBufferStr(q, " NOT NULL");
+		}
+	}
 
 	if (typdefault != NULL)
 	{
@@ -12518,7 +12531,7 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
 	{
 		ConstraintInfo *domcheck = &(tyinfo->domChecks[i]);
 
-		if (!domcheck->separate)
+		if (!domcheck->separate && domcheck->contype == 'c')
 			appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s",
 							  fmtId(domcheck->dobj.name), domcheck->condef);
 	}
@@ -18388,7 +18401,7 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
 										  .dropStmt = delq->data));
 		}
 	}
-	else if (coninfo->contype == 'c' && tbinfo == NULL)
+	else if (tbinfo == NULL)
 	{
 		/* CHECK constraint on a domain */
 		TypeInfo   *tyinfo = coninfo->condomain;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 386e21e0c59..71ad355b97c 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1205,6 +1205,22 @@ my %tests = (
 		},
 	},
 
+	'DOMAIN CONSTRAINT NOT NULL / NOT VALID' => {
+		create_sql => 'CREATE DOMAIN dump_test.test_domain_nn AS INT;
+					   ALTER DOMAIN dump_test.test_domain_nn ADD CONSTRAINT nn NOT NULL NOT VALID;',
+		regexp => qr/^
+			\QALTER DOMAIN dump_test.test_domain_nn\E \n^\s+
+			\QADD CONSTRAINT nn NOT NULL NOT VALID;\E
+			/xm,
+		like => {
+			%full_runs, %dump_test_schema_runs, section_post_data => 1,
+		},
+		unlike => {
+			exclude_dump_test_schema => 1,
+			only_dump_measurement => 1,
+		},
+	},
+
 	'CONSTRAINT NOT NULL / NOT VALID (child1)' => {
 		regexp => qr/^
 		\QCREATE TABLE dump_test.test_table_nn_chld1 (\E\n
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 1d08268393e..f7c6d56fc01 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -4558,7 +4558,9 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
 					  "       pg_catalog.format_type(t.typbasetype, t.typtypmod) as \"%s\",\n"
 					  "       (SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type bt\n"
 					  "        WHERE c.oid = t.typcollation AND bt.oid = t.typbasetype AND t.typcollation <> bt.typcollation) as \"%s\",\n"
-					  "       CASE WHEN t.typnotnull THEN 'not null' END as \"%s\",\n"
+					  "       CASE WHEN t.typnotnull THEN "
+					  "       (SELECT lower(pg_catalog.pg_get_constraintdef(r.oid, true)) FROM pg_catalog.pg_constraint r WHERE t.oid = r.contypid AND r.contype = " CppAsString2(CONSTRAINT_NOTNULL) ")"
+					  "       END as \"%s\",\n"
 					  "       t.typdefault as \"%s\",\n"
 					  "       pg_catalog.array_to_string(ARRAY(\n"
 					  "         SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM pg_catalog.pg_constraint r WHERE t.oid = r.contypid AND r.contype = " CppAsString2(CONSTRAINT_CHECK) " ORDER BY r.conname\n"
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index ba6f05eeb7d..36ab8b5901b 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -926,6 +926,47 @@ ALTER DOMAIN things VALIDATE CONSTRAINT meow;
 ERROR:  column "stuff" of table "thethings" contains values that violate the new constraint
 UPDATE thethings SET stuff = 10;
 ALTER DOMAIN things VALIDATE CONSTRAINT meow;
+SELECT * FROM thethings ORDER BY 1;
+ stuff 
+-------
+    10
+(1 row)
+
+ALTER DOMAIN things ADD CONSTRAINT nn1 NOT NULL;
+ALTER DOMAIN things ADD CONSTRAINT domain_nn NOT NULL NOT VALID; --no-op
+ALTER DOMAIN things DROP NOT NULL;
+INSERT INTO thethings VALUES(NULL);
+ALTER DOMAIN things ADD CONSTRAINT domain_nn NOT NULL NOT VALID; --ok
+INSERT INTO thethings VALUES(NULL); --error
+ERROR:  domain things does not allow null values
+ALTER DOMAIN things ADD CONSTRAINT nn1 NOT NULL; --error
+ERROR:  incompatible NOT VALID constraint "domain_nn" on domain "things"
+HINT:  You might need to validate it using ALTER DOMAIN ... VALIDATE CONSTRAINT.
+ALTER DOMAIN things SET NOT NULL; --error
+ERROR:  column "stuff" of table "thethings" contains null values
+ALTER DOMAIN things ADD CONSTRAINT domain_nn1 NOT NULL NOT VALID; --no-op
+\dD things
+                                      List of domains
+ Schema |  Name  |  Type   | Collation |      Nullable      | Default |       Check        
+--------+--------+---------+-----------+--------------------+---------+--------------------
+ public | things | integer |           | not null not valid |         | CHECK (VALUE < 11)
+(1 row)
+
+SELECT conname, pg_get_constraintdef(oid)
+FROM pg_constraint
+WHERE contypid = 'things'::regtype
+ORDER BY conname COLLATE "C";
+  conname  | pg_get_constraintdef 
+-----------+----------------------
+ domain_nn | NOT NULL NOT VALID
+ meow      | CHECK ((VALUE < 11))
+(2 rows)
+
+ALTER DOMAIN things VALIDATE CONSTRAINT domain_nn; --error
+ERROR:  column "stuff" of table "thethings" contains null values
+UPDATE thethings SET stuff = 10 WHERE stuff IS NULL;
+ALTER DOMAIN things SET NOT NULL; --ok
+ALTER DOMAIN things DROP NOT NULL; --ok
 -- Confirm ALTER DOMAIN with RULES.
 create table domtab (col1 integer);
 create domain dom as integer;
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index b752a63ab5f..f10b80cc142 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -536,6 +536,28 @@ ALTER DOMAIN things ADD CONSTRAINT meow CHECK (VALUE < 11) NOT VALID;
 ALTER DOMAIN things VALIDATE CONSTRAINT meow;
 UPDATE thethings SET stuff = 10;
 ALTER DOMAIN things VALIDATE CONSTRAINT meow;
+SELECT * FROM thethings ORDER BY 1;
+ALTER DOMAIN things ADD CONSTRAINT nn1 NOT NULL;
+ALTER DOMAIN things ADD CONSTRAINT domain_nn NOT NULL NOT VALID; --no-op
+ALTER DOMAIN things DROP NOT NULL;
+
+INSERT INTO thethings VALUES(NULL);
+ALTER DOMAIN things ADD CONSTRAINT domain_nn NOT NULL NOT VALID; --ok
+INSERT INTO thethings VALUES(NULL); --error
+ALTER DOMAIN things ADD CONSTRAINT nn1 NOT NULL; --error
+ALTER DOMAIN things SET NOT NULL; --error
+ALTER DOMAIN things ADD CONSTRAINT domain_nn1 NOT NULL NOT VALID; --no-op
+
+\dD things
+SELECT conname, pg_get_constraintdef(oid)
+FROM pg_constraint
+WHERE contypid = 'things'::regtype
+ORDER BY conname COLLATE "C";
+
+ALTER DOMAIN things VALIDATE CONSTRAINT domain_nn; --error
+UPDATE thethings SET stuff = 10 WHERE stuff IS NULL;
+ALTER DOMAIN things SET NOT NULL; --ok
+ALTER DOMAIN things DROP NOT NULL; --ok
 
 -- Confirm ALTER DOMAIN with RULES.
 create table domtab (col1 integer);
-- 
2.34.1