From 5a4f7792a764c6c160cbbdd2726ddda9f9573b1a Mon Sep 17 00:00:00 2001 From: jian he Date: Wed, 24 Sep 2025 11:07:40 +0800 Subject: [PATCH] ALTER DOMAIN ADD NOT NULL NOT VALID We already support NOT NULL NO VALID for table constraints, and the same can be extended to domains by allowing them to have NOT VALID not-null constraints. ALTER DOMAIN SET NOT NULL: works similar to ALTER TABLE, it will validates an existing NOT VALID not-null domain constraint and updates pg_constraint.convalidated to true. ALTER DOMAIN ADD NOT NULL: creates a new, valid not-null constraint. If the domain already has a NOT VALID not-null constraint, the command raises an error. If the domain already has a valid not-null constraint, adding a NOT VALID not-null constraint is a no-op. \dD changes: for domain's not null not valid constraint: now column "Nullable" will display as "not null not valid". valid domain not-null constraint works as before. discussion: https://postgr.es/m/CACJufxGcABLgmH951SJkkihK+FW8KR3=odBhXEVCF9atQbur2Q@mail.gmail.com relate discussion: https://postgr.es/m/CACJufxE2oFcLmrqDrqJrH5k03fv+v9=+-PBs-mV5WsJ=31XMyw@mail.gmail.com commitfest entry: https://commitfest.postgresql.org/patch/5768 --- doc/src/sgml/catalogs.sgml | 2 +- doc/src/sgml/ref/alter_domain.sgml | 1 - src/backend/catalog/pg_constraint.c | 8 +- src/backend/commands/typecmds.c | 139 ++++++++++++++++++++++++--- src/backend/parser/gram.y | 8 +- src/bin/pg_dump/t/002_pg_dump.pl | 16 +++ src/bin/psql/describe.c | 4 +- src/test/regress/expected/domain.out | 40 ++++++++ src/test/regress/sql/domain.sql | 22 +++++ 9 files changed, 214 insertions(+), 26 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 2fc634429802..7626321812b4 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -9569,7 +9569,7 @@ SCRAM-SHA-256$<iteration count>:&l typnotnull represents a not-null - constraint on a type. Used for domains only. + (possibly invalid) constraint on a type. Used for domains only. diff --git a/doc/src/sgml/ref/alter_domain.sgml b/doc/src/sgml/ref/alter_domain.sgml index 74855172222e..4807116eb053 100644 --- a/doc/src/sgml/ref/alter_domain.sgml +++ b/doc/src/sgml/ref/alter_domain.sgml @@ -92,7 +92,6 @@ ALTER DOMAIN name valid using ALTER DOMAIN ... VALIDATE CONSTRAINT. Newly inserted or updated rows are always checked against all constraints, even those marked NOT VALID. - NOT VALID is only accepted for CHECK constraints. diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 672b188930f3..d37553156532 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -651,7 +651,7 @@ findNotNullConstraint(Oid relid, const char *colname) } /* - * Find and return the pg_constraint tuple that implements a validated + * Find and return the pg_constraint tuple that implements (possibly not valid) * not-null constraint for the given domain. */ HeapTuple @@ -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 NOT NULL 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 be6ffd6ddb09..0b9ba2de76a3 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -127,7 +127,7 @@ static Oid findRangeSubOpclass(List *opcname, Oid subtype); static Oid findRangeCanonicalFunction(List *procname, Oid typeOid); static Oid findRangeSubtypeDiffFunction(List *procname, Oid subtype); static void validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmode); -static void validateDomainNotNullConstraint(Oid domainoid); +static void validateDomainNotNullConstraint(Oid domainoid, LOCKMODE lockmode); static List *get_rels_with_domain(Oid domainOid, LOCKMODE lockmode); static void checkEnumOwner(HeapTuple tup); static char *domainAddCheckConstraint(Oid domainOid, Oid domainNamespace, @@ -2768,12 +2768,71 @@ 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; } + /* + * We may need to validate existing NOT VALID not-null constraint + */ + 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); + + 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 existing NOT VALID + * constraint, also set the pg_constraint.convalidated to true. + */ + if (!con->convalidated) + { + validateDomainNotNullConstraint(domainoid, ShareUpdateExclusiveLock); + copyTuple = heap_copytuple(conTup); + + copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple); + copy_con->convalidated = true; + CatalogTupleUpdate(pg_constraint, ©Tuple->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 the existing NOT VALID not-null constraint has already been + * validated, there is no need to add another one, exit now. + */ + return address; + } + if (notNull) { Constraint *constr; @@ -2787,7 +2846,7 @@ AlterDomainNotNull(List *names, bool notNull) typTup->typbasetype, typTup->typtypmod, constr, NameStr(typTup->typname), NULL); - validateDomainNotNullConstraint(domainoid); + validateDomainNotNullConstraint(domainoid, ShareLock); } else { @@ -3001,18 +3060,57 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, } else if (constr->contype == CONSTR_NOTNULL) { - /* Is the domain already set NOT NULL? */ + /* Is the domain already have a NOT NULL constraint? */ if (typTup->typnotnull) { + HeapTuple conTup; + ScanKeyData key[1]; + SysScanDesc scan; + Relation pg_constraint; + + pg_constraint = table_open(ConstraintRelationId, AccessShareLock); + + 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 another valid not-null constraint if the domain + * already have a NOT VALID one. + */ + if (!con->convalidated && constr->initially_valid) + 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); table_close(typrel, RowExclusiveLock); + return address; } + domainAddNotNullConstraint(domainoid, typTup->typnamespace, typTup->typbasetype, typTup->typtypmod, constr, NameStr(typTup->typname), constrAddr); if (!constr->skip_validation) - validateDomainNotNullConstraint(domainoid); + validateDomainNotNullConstraint(domainoid, ShareLock); typTup->typnotnull = true; CatalogTupleUpdate(typrel, &tup->t_self, tup); @@ -3092,21 +3190,27 @@ 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); - /* * Locking related relations with ShareUpdateExclusiveLock is ok because * not-yet-valid constraints are still enforced against concurrent inserts * or updates. - */ - validateDomainCheckConstraint(domainoid, conbin, ShareUpdateExclusiveLock); + */ + if (con->contype == CONSTRAINT_CHECK) + { + val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin); + + conbin = TextDatumGetCString(val); + + validateDomainCheckConstraint(domainoid, conbin, ShareUpdateExclusiveLock); + } + else + validateDomainNotNullConstraint(domainoid, ShareUpdateExclusiveLock); /* * Now update the catalog, while we have the door open. @@ -3134,9 +3238,16 @@ AlterDomainValidateConstraint(List *names, const char *constrName) /* * Verify that all columns currently using the domain are not null. + * + * It is used to validate existing not-null constraint and to add newly created + * not-null constraints to a domain. + * + * The lockmode is used for relations using the domain. It should be + * ShareLock when adding a new not-null to domain. It can be + * ShareUpdateExclusiveLock when validating the existing not-null constraint. */ static void -validateDomainNotNullConstraint(Oid domainoid) +validateDomainNotNullConstraint(Oid domainoid, LOCKMODE lockmode) { List *rels; ListCell *rt; @@ -3144,7 +3255,7 @@ validateDomainNotNullConstraint(Oid domainoid) /* Fetch relation list with attributes based on this domain */ /* ShareLock is sufficient to prevent concurrent data changes */ - rels = get_rels_with_domain(domainoid, ShareLock); + rels = get_rels_with_domain(domainoid, lockmode); foreach(rt, rels) { diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 28f4e11e30ff..3967a5099216 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4512,11 +4512,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/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index e33aa95f6ffc..12706b60c4e2 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1026,6 +1026,22 @@ }, }, + '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 36f245028429..2cdb2c705c50 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -4593,7 +4593,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 62a48a523a2d..e4bd99c8c159 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -927,6 +927,46 @@ 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 VALIDATE CONSTRAINT domain_nn; --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 and contype = 'n'; + conname | pg_get_constraintdef +-----------+---------------------- + domain_nn | NOT NULL NOT VALID +(1 row) + +UPDATE thethings SET stuff = 10 WHERE stuff IS NULL; +ALTER DOMAIN things SET NOT NULL; --ok +ALTER DOMAIN things VALIDATE CONSTRAINT domain_nn; --ok +ALTER DOMAIN things DROP NOT NULL; -- 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 b8f5a6397121..394f89b13aef 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 VALIDATE CONSTRAINT domain_nn; --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 and contype = 'n'; + +UPDATE thethings SET stuff = 10 WHERE stuff IS NULL; +ALTER DOMAIN things SET NOT NULL; --ok +ALTER DOMAIN things VALIDATE CONSTRAINT domain_nn; --ok +ALTER DOMAIN things DROP NOT NULL; -- Confirm ALTER DOMAIN with RULES. create table domtab (col1 integer);