From 99f8cbb9144ad3a6deb5ed110b3426efa278f522 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Mon, 28 Sep 2026 15:05:36 -0300 Subject: [PATCH v2 2/3] Validate re-added domain constraints after ALTER TABLE rewrites When ALTER TABLE ... ALTER COLUMN TYPE (or ALTER TYPE ... ALTER ATTRIBUTE) rebuilds a domain CHECK constraint whose expression depends on the altered column, the constraint was re-added through AlterDomainAddConstraint(), which validates it immediately against all columns of the domain. That happens during Phase 2, before Phase 3 has rewritten the affected tables, so any table that is pending a rewrite and has a column of the domain was scanned using its new tuple descriptor over its old heap. This could produce garbage values, spurious "contains values that violate the new constraint" errors, or worse, e.g. "type with OID 4294967295 does not exist" when the domain is over a composite type. Fix by skipping validation in AlterDomainAddConstraint() when re-adding a constraint, and instead having ATExecCmd() remember the rebuilt constraint so that ATRewriteTables() validates it once all tables have been rewritten. Constraints that were NOT VALID are not validated, as before. This problem dates back to af20e2d72, which added rebuilding of domain constraints, but was previously only reachable with domains over composite types, since other domains hit the "could not identify relation associated with constraint" error instead. --- src/backend/commands/tablecmds.c | 56 ++++++++++++++-- src/backend/commands/typecmds.c | 11 ++-- src/include/commands/typecmds.h | 1 + src/test/regress/expected/domain.out | 99 ++++++++++++++++++++++++++++ src/test/regress/sql/domain.sql | 63 ++++++++++++++++++ 5 files changed, 221 insertions(+), 9 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c8bc193a2ab..10c7a3a5f8e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -198,6 +198,8 @@ typedef struct AlteredTableInfo bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */ char newrelpersistence; /* if above is true */ Expr *partition_constraint; /* for attach partition validation */ + /* OIDs of re-added domain CHECK constraints to validate in Phase 3 */ + List *domain_constraints; /* true, if validating default due to some other attach/detach */ bool validate_default; /* Objects to rebuild after completing ALTER TYPE operations */ @@ -5535,11 +5537,25 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_ReAddDomainConstraint: /* Re-add pre-existing domain check * constraint */ - address = - AlterDomainAddConstraint(((AlterDomainStmt *) cmd->def)->typeName, - ((AlterDomainStmt *) cmd->def)->def, - NULL, true); - break; + { + AlterDomainStmt *stmt = (AlterDomainStmt *) cmd->def; + Constraint *con = castNode(Constraint, stmt->def); + ObjectAddress constrAddr; + + address = AlterDomainAddConstraint(stmt->typeName, stmt->def, + &constrAddr, true); + + /* + * AlterDomainAddConstraint doesn't validate re-added + * constraints, since tables using the domain may not have + * been rewritten yet. Tell Phase 3 to do it. + */ + if (!con->skip_validation) + tab->domain_constraints = + lappend_oid(tab->domain_constraints, + constrAddr.objectId); + break; + } case AT_ReAddComment: /* Re-add existing comment */ address = CommentObject((CommentStmt *) cmd->def); break; @@ -6160,6 +6176,36 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, table_close(rel, NoLock); } + /* + * Validate re-added domain CHECK constraints. This must wait until all + * tables have been rewritten, since any of them might contain columns of + * the domain. Don't skip relations without storage since the work queue + * entry might be for a standalone composite type. + */ + foreach(ltab, *wqueue) + { + AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab); + + foreach_oid(conoid, tab->domain_constraints) + { + HeapTuple tup; + Form_pg_constraint con; + Datum conbin; + + tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(conoid)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for constraint %u", conoid); + con = (Form_pg_constraint) GETSTRUCT(tup); + + conbin = SysCacheGetAttrNotNull(CONSTROID, tup, + Anum_pg_constraint_conbin); + validateDomainCheckConstraint(con->contypid, + TextDatumGetCString(conbin)); + + ReleaseSysCache(tup); + } + } + /* Finally, run any afterStmts that were queued up */ foreach(ltab, *wqueue) { diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 99a0ae2228e..d0349079a1e 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -128,7 +128,6 @@ static Oid findTypeSubscriptingFunction(List *procname, Oid typeOid); 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); static void validateDomainNotNullConstraint(Oid domainoid); static List *get_rels_with_domain(Oid domainOid, LOCKMODE lockmode); static void checkEnumOwner(HeapTuple tup); @@ -3029,13 +3028,17 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, constr, NameStr(typTup->typname), constrAddr, is_readd); - /* * If requested to validate the constraint, test all values stored in * the attributes based on the domain the constraint is being added * to. + * + * When re-adding a constraint during ALTER TABLE, the tables using + * the domain might not have been rewritten to match their new + * catalog definitions yet, so the caller must do the validation after + * its rewrite phase instead. */ - if (!constr->skip_validation) + if (!constr->skip_validation && !is_readd) validateDomainCheckConstraint(domainoid, ccbin); /* @@ -3249,7 +3252,7 @@ validateDomainNotNullConstraint(Oid domainoid) * Verify that all columns currently using the domain satisfy the given check * constraint expression. */ -static void +void validateDomainCheckConstraint(Oid domainoid, const char *ccbin) { Expr *expr = (Expr *) stringToNode(ccbin); diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h index 2112b4addd2..a067651f6f9 100644 --- a/src/include/commands/typecmds.h +++ b/src/include/commands/typecmds.h @@ -38,6 +38,7 @@ extern ObjectAddress AlterDomainAddConstraint(List *names, Node *newConstraint, ObjectAddress *constrAddr, bool is_readd); extern ObjectAddress AlterDomainValidateConstraint(List *names, const char *constrName); +extern void validateDomainCheckConstraint(Oid domainoid, const char *ccbin); extern ObjectAddress AlterDomainDropConstraint(List *names, const char *constrName, DropBehavior behavior, bool missing_ok); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index de60a90c045..14f2c928700 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -482,6 +482,105 @@ ERROR: value for domain dt_multi violates check constraint "dt_multi_check" drop domain dt_multi; drop type r1 cascade; drop type r2 cascade; +-- A domain constraint rebuilt by ALTER COLUMN TYPE must not be validated +-- until tables using the domain have been rewritten. (These tests avoid +-- ROW(value)::domrw_t, since the rebuilt expression would then contain a +-- NULL coerced to the domain itself.) +create function domrw_show(int) returns bool language plpgsql as + $$ begin raise notice 'domain check sees value %', $1; return true; end $$; +create table domrw_t (c int); +create domain domrw_dt as int + check (domrw_show(value) and (null::domrw_t).c is null); +alter table domrw_t add column d domrw_dt; +insert into domrw_t values (1, 5), (2, 7); +NOTICE: domain check sees value 5 +NOTICE: domain check sees value 7 +alter table domrw_t alter column c type bigint; -- should see 5 and 7 +NOTICE: domain check sees value 5 +NOTICE: domain check sees value 7 +select * from domrw_t; + c | d +---+--- + 1 | 5 + 2 | 7 +(2 rows) + +select convalidated from pg_constraint where contypid = 'domrw_dt'::regtype; + convalidated +-------------- + t +(1 row) + +drop domain domrw_dt cascade; +NOTICE: drop cascades to column d of table domrw_t +drop table domrw_t; +-- same, domain over composite +create type domrw_ct as (j int); +create table domrw_t (c int); +create domain domrw_dt as domrw_ct + check (domrw_show((value).j) and (null::domrw_t).c is null); +alter table domrw_t add column d domrw_dt; +insert into domrw_t values (1, row(5)), (2, row(7)); +NOTICE: domain check sees value 5 +NOTICE: domain check sees value 7 +alter table domrw_t alter column c type bigint; -- should see 5 and 7 +NOTICE: domain check sees value 5 +NOTICE: domain check sees value 7 +select * from domrw_t; + c | d +---+----- + 1 | (5) + 2 | (7) +(2 rows) + +drop domain domrw_dt cascade; +NOTICE: drop cascades to column d of table domrw_t +drop table domrw_t; +drop type domrw_ct; +drop function domrw_show(int); +-- domain column in an inheritance child that is rewritten by recursion +create table domrw_p (c int); +create domain domrw_dt as int check ((row(value)::domrw_p).c > 0); +create table domrw_ch (d domrw_dt) inherits (domrw_p); +insert into domrw_ch values (1, 5), (2, 7); +alter table domrw_p alter column c type bigint; +select * from domrw_ch; + c | d +---+--- + 1 | 5 + 2 | 7 +(2 rows) + +drop domain domrw_dt cascade; +NOTICE: drop cascades to column d of table domrw_ch +drop table domrw_p cascade; +NOTICE: drop cascades to table domrw_ch +-- a rebuilt constraint that rejects stored values must still be enforced, +-- even when the altered object is a standalone composite type +create type domrw_rt as (i int); +create domain domrw_dt as int check ((row(value)::domrw_rt).i is not null); +create table domrw_u (x domrw_dt); +insert into domrw_u values (40000); +alter type domrw_rt alter attribute i type smallint; -- fail +ERROR: smallint out of range +drop table domrw_u; +-- a NOT VALID constraint is not validated and stays NOT VALID +alter domain domrw_dt drop constraint domrw_dt_check; +create table domrw_u (x domrw_dt); +insert into domrw_u values (40000); +alter domain domrw_dt add constraint domrw_nv + check ((row(value)::domrw_rt).i is not null) not valid; +alter type domrw_rt alter attribute i type smallint; +select pg_get_constraintdef(oid), convalidated from pg_constraint + where contypid = 'domrw_dt'::regtype; + pg_get_constraintdef | convalidated +----------------------------------------------------------------------+-------------- + CHECK (((ROW((VALUE)::smallint)::domrw_rt).i IS NOT NULL)) NOT VALID | f +(1 row) + +drop table domrw_u; +drop domain domrw_dt; +drop type domrw_rt; -- Test domains over arrays of composite create type comptype as (r float8, i float8); create domain dcomptypea as comptype[]; diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 1240f9422bd..b6e452d6ebb 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -252,6 +252,69 @@ drop domain dt_multi; drop type r1 cascade; drop type r2 cascade; +-- A domain constraint rebuilt by ALTER COLUMN TYPE must not be validated +-- until tables using the domain have been rewritten. (These tests avoid +-- ROW(value)::domrw_t, since the rebuilt expression would then contain a +-- NULL coerced to the domain itself.) +create function domrw_show(int) returns bool language plpgsql as + $$ begin raise notice 'domain check sees value %', $1; return true; end $$; +create table domrw_t (c int); +create domain domrw_dt as int + check (domrw_show(value) and (null::domrw_t).c is null); +alter table domrw_t add column d domrw_dt; +insert into domrw_t values (1, 5), (2, 7); +alter table domrw_t alter column c type bigint; -- should see 5 and 7 +select * from domrw_t; +select convalidated from pg_constraint where contypid = 'domrw_dt'::regtype; +drop domain domrw_dt cascade; +drop table domrw_t; + +-- same, domain over composite +create type domrw_ct as (j int); +create table domrw_t (c int); +create domain domrw_dt as domrw_ct + check (domrw_show((value).j) and (null::domrw_t).c is null); +alter table domrw_t add column d domrw_dt; +insert into domrw_t values (1, row(5)), (2, row(7)); +alter table domrw_t alter column c type bigint; -- should see 5 and 7 +select * from domrw_t; +drop domain domrw_dt cascade; +drop table domrw_t; +drop type domrw_ct; +drop function domrw_show(int); + +-- domain column in an inheritance child that is rewritten by recursion +create table domrw_p (c int); +create domain domrw_dt as int check ((row(value)::domrw_p).c > 0); +create table domrw_ch (d domrw_dt) inherits (domrw_p); +insert into domrw_ch values (1, 5), (2, 7); +alter table domrw_p alter column c type bigint; +select * from domrw_ch; +drop domain domrw_dt cascade; +drop table domrw_p cascade; + +-- a rebuilt constraint that rejects stored values must still be enforced, +-- even when the altered object is a standalone composite type +create type domrw_rt as (i int); +create domain domrw_dt as int check ((row(value)::domrw_rt).i is not null); +create table domrw_u (x domrw_dt); +insert into domrw_u values (40000); +alter type domrw_rt alter attribute i type smallint; -- fail +drop table domrw_u; + +-- a NOT VALID constraint is not validated and stays NOT VALID +alter domain domrw_dt drop constraint domrw_dt_check; +create table domrw_u (x domrw_dt); +insert into domrw_u values (40000); +alter domain domrw_dt add constraint domrw_nv + check ((row(value)::domrw_rt).i is not null) not valid; +alter type domrw_rt alter attribute i type smallint; +select pg_get_constraintdef(oid), convalidated from pg_constraint + where contypid = 'domrw_dt'::regtype; +drop table domrw_u; +drop domain domrw_dt; +drop type domrw_rt; + -- Test domains over arrays of composite -- 2.50.1 (Apple Git-155)