hi. attached patch is for $subject implementation
per https://www.postgresql.org/docs/current/sql-alterdomain.html """ Although ALTER DOMAIN ADD CONSTRAINT attempts to verify that existing stored data satisfies the new constraint, this check is not bulletproof, because the command cannot “see” table rows that are newly inserted or updated and not yet committed. If there is a hazard that concurrent operations might insert bad data, the way to proceed is to add the constraint using the NOT VALID option, commit that command, wait until all transactions started before that commit have finished, and then issue ALTER DOMAIN VALIDATE CONSTRAINT to search for data violating the constraint. """ Obviously, the above behavior can also happen to not-null constraints. add NOT NULL NOT VALID is good for validation invalid data too. the not valid information is displayed at column "Nullable" for example: \dD things List of domains Schema | Name | Type | Collation | Nullable | Default | Check --------+--------+---------+-----------+--------------------+---------+-------------------- public | things | integer | | not null not valid | | CHECK (VALUE < 11)
From bdea7a60eab6e46d7f0b59b9a5602d6a2421f60d Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Wed, 21 May 2025 18:43:17 +0800 Subject: [PATCH v1 1/1] ALTER DOMAIN ADD NOT NULL NOT VALID we have NOT NULL NO VALID for table constraints, now 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/ --- 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 | 115 +++++++++++++++++++++++++-- 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 | 40 ++++++++++ src/test/regress/sql/domain.sql | 21 +++++ 10 files changed, 216 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><iteration count></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..15549673f1e 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 && !notNull) { table_close(typrel, RowExclusiveLock); return address; } + if (typTup->typnotnull == notNull && 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 row 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, ©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 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,44 @@ 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 valid not-null if domain already have NOT VALID NOT NULL. + */ + 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 +3176,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 c73e73a87d1..4ebc96f8564 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; @@ -12484,8 +12487,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) { @@ -12505,7 +12518,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); } @@ -18375,7 +18388,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 cf34f71ea11..f15d22395c6 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1146,6 +1146,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..c4863ba9b53 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -926,6 +926,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 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; + conname | pg_get_constraintdef +-----------+---------------------- + meow | CHECK ((VALUE < 11)) + domain_nn | NOT NULL NOT VALID +(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..9b5a76af48b 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -536,6 +536,27 @@ 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; + +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
