From b8a90f1c6a6c2a2d5730504e4969245cb3b64215 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Mon, 28 Sep 2026 15:05:36 -0300 Subject: [PATCH v4 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 | 59 +++++++++++++++-- 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, 224 insertions(+), 9 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 07cf49b286a..cad92dd8577 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,28 @@ 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 = InvalidObjectAddress; + + /* only CHECK constraints can depend on a column */ + Assert(con->contype == CONSTR_CHECK); + + 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 +6179,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 6f3b59b5181..9042ded6078 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -506,6 +506,105 @@ ERROR: value for domain dt_inh violates check constraint "dt_inh_check" drop domain dt_inh; drop table dc; drop table dp; +-- 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 f8cf8383667..b791691f560 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -267,6 +267,69 @@ drop domain dt_inh; drop table dc; drop table dp; +-- 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.56.0.rc1.315.gc6ed9934b7-goog