On Thu, May 22, 2025 at 10:08 PM Quan Zongliang <quanzongli...@yeah.net> wrote: > > It makes sense to support the "NOT NULL NOT VALID" option. > > The two if statements in the AlterDomainNotNull() should be adjusted. > > if (typTup->typnotnull == notNull && !notNull) > ==> > if (!notNull && !typTup->typnotnull) > > > if (typTup->typnotnull == notNull && notNull) > ==> > if (notNull && typTup->typnotnull) >
yech, you are right. There is one failed test [0] because of query result order, so I add "ORDER BY conname COLLATE "C";" to stabilize tests. [0] https://api.cirrus-ci.com/v1/artifact/task/6676676240736256/testrun/build/testrun/pg_upgrade/002_pg_upgrade/data/regression.diffs
From a840278f579b57cab4c13b91abfcc4f44a6d8a83 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@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=odbhxevcf9atqbu...@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><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..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, ©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,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
