I'm adding the v4 patches. Matheus's 0002 and 0003 are rebased on top of 0001.
I've amended the commit message to include ALTER TABLE, ALTER COLUMN etc. Have edited some of the comments in the test. Have also added a comment to the actual code explaining why tab->relid is safe. I haven't removed tests but made some changes like removing cascade from one of them. Have changed the test with multiple relations to now have one invalid alter. Mainly I wanted to test that after the first ALTER succeeds and recreates the constraint, the constraint is correctly recreated for the other relation and still fails if an incompatible type is used. I kept one of the tests with float8 because it was mentioned in the email thread for the original commit. If reviewers think fewer tests are better, I can drop the test. On Tue, Sep 29, 2026 at 7:25 PM Matheus Alcantara <[email protected]> wrote: > But I don't think that's enough. The underlying issue is that the re-add > looks the domain up by the name saved in its definition, so the name can > point to a different type by the time the constraint is re-added. In > your isolation test example, if c_swap creates a new domain instead > (CREATE DOMAIN d AS ct), the type check passes, the ALTER succeeds, and > d_check silently ends up on the new domain, while the original (now > d_old) loses it. > > Note that this isn't new with the patch. What 0003 changes is that > without the ownership check, it also works when the domain belongs to > someone other than the user running the ALTER. > > We may try to capture the domain oid above AlterDomainAddConstraint, > while the old constraint still exists and re-add the constraint to that > OID instead of resolving the name again. But I think that it will > require more code to write which would make it harder for back patching. > Looking for thoughts here. > Thanks for pointing this out. Perhaps this can be done in a separate patch without back-patching. Regards, Nitin Motiani Google
From 9e9d4532fc2dd12a1ac367d0ad1d1e74cb318dcf Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Mon, 28 Sep 2026 15:18:56 -0300 Subject: [PATCH v4 3/3] Don't require ownership when rebuilding constraints in ALTER TABLE When ALTER TABLE ... ALTER COLUMN TYPE (or ALTER TYPE ... ALTER ATTRIBUTE) rebuilds a constraint that depends on the altered column, it drops the constraint and re-creates it from its saved definition. For domain CHECK constraints the re-creation goes through AlterDomainAddConstraint(), which calls checkDomainOwner(), and any comment on a table or domain constraint is restored with CommentObject(), which requires ownership of the constraint's table or domain. So a user altering a type or table they own would fail with "must be owner of type ..." or "must be owner of relation ..." if another user's domain or table has a constraint that depends on it. Since types grant USAGE to PUBLIC by default, any user could create such a dependency and block the owner from altering their own type. These checks don't make sense here: the user isn't choosing to add a constraint or comment, just restoring ones that already existed, and the matching drop is already done without any permission checks. Rebuilding a table constraint without a comment also doesn't check ownership. Fix by skipping the ownership check in AlterDomainAddConstraint() when is_readd is set, as we already do for the USAGE check on types used by the expression, and by restoring comments directly with CreateComments() instead of CommentObject(). The domain part of this dates back to af20e2d72, which added rebuilding of domain constraints. --- src/backend/commands/tablecmds.c | 22 +++++++++++++++- src/backend/commands/typecmds.c | 24 ++++++++++++++---- src/test/regress/expected/domain.out | 38 ++++++++++++++++++++++++++++ src/test/regress/sql/domain.sql | 31 +++++++++++++++++++++++ 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cad92dd8577..a042b1fabf7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5560,7 +5560,27 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; } case AT_ReAddComment: /* Re-add existing comment */ - address = CommentObject((CommentStmt *) cmd->def); + { + CommentStmt *stmt = (CommentStmt *) cmd->def; + Relation comrel; + + /* + * Don't use CommentObject(), since that requires ownership of + * the constraint's table or domain, which the user altering a + * column the constraint depends on need not have. We're just + * restoring a comment that already existed. + */ + Assert(stmt->objtype == OBJECT_TABCONSTRAINT || + stmt->objtype == OBJECT_DOMCONSTRAINT); + address = get_object_address(stmt->objtype, stmt->object, + &comrel, + ShareUpdateExclusiveLock, + false); + CreateComments(address.objectId, address.classId, + address.objectSubId, stmt->comment); + if (comrel != NULL) + relation_close(comrel, NoLock); + } break; case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ address = ATExecAddIndexConstraint(tab, rel, (IndexStmt *) cmd->def, diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index d0349079a1e..7e9846bda0e 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -3004,8 +3004,22 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, elog(ERROR, "cache lookup failed for type %u", domainoid); typTup = (Form_pg_type) GETSTRUCT(tup); - /* Check it's a domain and check user has permission for ALTER DOMAIN */ - checkDomainOwner(tup); + /* + * Check it's a domain and check user has permission for ALTER DOMAIN. + * When re-adding a constraint during ALTER TABLE, skip the permission + * check since the constraint already existed, and the user altering a + * column it depends on need not own the domain. + */ + if (is_readd) + { + if (typTup->typtype != TYPTYPE_DOMAIN) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("%s is not a domain", + format_type_be(typTup->oid)))); + } + else + checkDomainOwner(tup); if (!IsA(newConstraint, Constraint)) elog(ERROR, "unrecognized node type: %d", @@ -3034,9 +3048,9 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, * 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. + * 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 && !is_readd) validateDomainCheckConstraint(domainoid, ccbin); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index 9042ded6078..35a2e74b6ff 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -605,6 +605,44 @@ select pg_get_constraintdef(oid), convalidated from pg_constraint drop table domrw_u; drop domain domrw_dt; drop type domrw_rt; +-- Rebuilding a constraint (and its comment) owned by someone else must not +-- require ownership of the constraint's domain or table +create role regress_domrw_typeowner; +create role regress_domrw_conowner; +grant create on schema public to regress_domrw_typeowner, regress_domrw_conowner; +set role regress_domrw_typeowner; +create type domrw_rt as (i int); +set role regress_domrw_conowner; +create domain domrw_dt1 as int + constraint domrw_dt1_check check ((row(value)::domrw_rt).i > 0); +comment on constraint domrw_dt1_check on domain domrw_dt1 is 'domain over int'; +create domain domrw_dt2 as domrw_rt + constraint domrw_dt2_check check ((value).i > 0); +comment on constraint domrw_dt2_check on domain domrw_dt2 is 'domain over composite'; +create table domrw_t (x int + constraint domrw_t_check check ((row(x)::domrw_rt).i > 0)); +comment on constraint domrw_t_check on domrw_t is 'table constraint'; +set role regress_domrw_typeowner; +alter type domrw_rt alter attribute i type bigint; +reset role; +select conname, pg_get_constraintdef(oid), obj_description(oid, 'pg_constraint') + from pg_constraint where conname like 'domrw\_%' order by conname; + conname | pg_get_constraintdef | obj_description +-----------------+--------------------------------------------------+----------------------- + domrw_dt1_check | CHECK (((ROW((VALUE)::bigint)::domrw_rt).i > 0)) | domain over int + domrw_dt2_check | CHECK (((VALUE).i > 0)) | domain over composite + domrw_t_check | CHECK (((ROW((x)::bigint)::domrw_rt).i > 0)) | table constraint +(3 rows) + +select (-1)::domrw_dt1; -- fail +ERROR: value for domain domrw_dt1 violates check constraint "domrw_dt1_check" +drop table domrw_t; +drop domain domrw_dt1; +drop domain domrw_dt2; +drop type domrw_rt; +revoke create on schema public from regress_domrw_typeowner, regress_domrw_conowner; +drop role regress_domrw_typeowner; +drop role regress_domrw_conowner; -- 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 b791691f560..b0cfeb002df 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -330,6 +330,37 @@ drop table domrw_u; drop domain domrw_dt; drop type domrw_rt; +-- Rebuilding a constraint (and its comment) owned by someone else must not +-- require ownership of the constraint's domain or table +create role regress_domrw_typeowner; +create role regress_domrw_conowner; +grant create on schema public to regress_domrw_typeowner, regress_domrw_conowner; +set role regress_domrw_typeowner; +create type domrw_rt as (i int); +set role regress_domrw_conowner; +create domain domrw_dt1 as int + constraint domrw_dt1_check check ((row(value)::domrw_rt).i > 0); +comment on constraint domrw_dt1_check on domain domrw_dt1 is 'domain over int'; +create domain domrw_dt2 as domrw_rt + constraint domrw_dt2_check check ((value).i > 0); +comment on constraint domrw_dt2_check on domain domrw_dt2 is 'domain over composite'; +create table domrw_t (x int + constraint domrw_t_check check ((row(x)::domrw_rt).i > 0)); +comment on constraint domrw_t_check on domrw_t is 'table constraint'; +set role regress_domrw_typeowner; +alter type domrw_rt alter attribute i type bigint; +reset role; +select conname, pg_get_constraintdef(oid), obj_description(oid, 'pg_constraint') + from pg_constraint where conname like 'domrw\_%' order by conname; +select (-1)::domrw_dt1; -- fail +drop table domrw_t; +drop domain domrw_dt1; +drop domain domrw_dt2; +drop type domrw_rt; +revoke create on schema public from regress_domrw_typeowner, regress_domrw_conowner; +drop role regress_domrw_typeowner; +drop role regress_domrw_conowner; + -- Test domains over arrays of composite -- 2.56.0.rc1.315.gc6ed9934b7-goog
From 934c6a653dc823d9dc328880b173994a7fb70376 Mon Sep 17 00:00:00 2001 From: Nitin Motiani <[email protected]> Date: Mon, 28 Sep 2026 12:49:33 +0000 Subject: [PATCH v4 1/3] Fix ALTER ... TYPE failure with dependent domain constraints. When ALTER TYPE ... ALTER ATTRIBUTE ... TYPE or ALTER TABLE ... ALTER COLUMN ... TYPE changes a column that a domain's CHECK constraint depends on, ATPostAlterTypeCleanup() arranges for the constraint to be dropped and re-created. Since commit af20e2d72, it chose the relation to attach the re-creation step to by calling get_typ_typrelid(getBaseType(con->contypid)), which only works if the domain's base type is composite. For a domain whose base type isn't composite (for example int, float8 or int[], possibly via another domain) but whose CHECK expression references a composite type or a table's row type, that returned InvalidOid, and the command failed with "could not identify relation associated with constraint". For a domain constraint, that relation OID only decides whether another relation gets locked and which work queue entry receives the AT_ReAddDomainConstraint command; the command itself identifies the domain by name. So just use the relation being altered (tab->relid), which is already locked. A side effect is that for a domain over a composite type other than the one being altered, we no longer lock that other type's relation or add a work queue entry for it, neither of which was needed. Bug: #19724 Reported-by: Alexander Lakhin <[email protected]> Author: Nitin Motiani <[email protected]> Reviewed-by: Ayush Tiwari <[email protected]> Reviewed-by: Rahul Yadav <[email protected]> Reviewed-by: Matheus Alcantara <[email protected]> --- src/backend/commands/tablecmds.c | 12 +++-- src/test/regress/expected/domain.out | 74 ++++++++++++++++++++++++++++ src/test/regress/sql/domain.sql | 48 ++++++++++++++++++ 3 files changed, 130 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0274d892f2e..07cf49b286a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16141,10 +16141,14 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) relid = con->conrelid; else { - /* must be a domain constraint */ - relid = get_typ_typrelid(getBaseType(con->contypid)); - if (!OidIsValid(relid)) - elog(ERROR, "could not identify relation associated with constraint %u", oldId); + /* + * Must be a domain constraint. The domain's base type need not + * be composite, so there may be no relation associated with it. + * Since the relid is only used to determine which work queue + * entry the command is attached to, we can use the relation which + * is being altered and which we already hold a lock on. + */ + relid = tab->relid; } confrelid = con->confrelid; conislocal = con->conislocal; diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index 62a48a523a2..6f3b59b5181 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -432,6 +432,80 @@ select conname, obj_description(oid, 'pg_constraint') from pg_constraint drop type comptype cascade; NOTICE: drop cascades to type dcomptype +-- check altering columns used by constraints of domains whose base type +-- isn't composite (bug #19724) +create type rt as (i int); +create domain dt as int check ((row(value)::rt).i > 0); +alter type rt alter attribute i type text; -- fail +ERROR: operator does not exist: text > integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +alter type rt alter attribute i type bigint; +select 1::dt; + dt +---- + 1 +(1 row) + +select (-1)::dt; -- fail +ERROR: value for domain dt violates check constraint "dt_check" +drop domain dt; +drop type rt; +-- same for a domain over float8 that uses one field of a two-field type +create type comptype as (r float8, i float8); +create domain silly as float8 check ((row(value, 0)::comptype).r > 0); +alter type comptype alter attribute r type bigint; +select 1.0::silly; + silly +------- + 1 +(1 row) + +select (-1.0)::silly; -- fail +ERROR: value for domain silly violates check constraint "silly_check" +drop domain silly; +drop type comptype; +-- rebuilding a constraint for one type it depends on must preserve its +-- dependency on another +create type r1 as (a int); +create type r2 as (b int); +create domain dt_multi as int + check ((row(value)::r1).a > 0 and (row(value)::r2).b > 0); +alter type r1 alter attribute a type bigint; +alter type r2 alter attribute b type text; -- fail +ERROR: operator does not exist: text > integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +alter type r2 alter attribute b type bigint; +select 1::dt_multi; + dt_multi +---------- + 1 +(1 row) + +select (-1)::dt_multi; -- fail +ERROR: value for domain dt_multi violates check constraint "dt_multi_check" +drop domain dt_multi; +drop type r1; +drop type r2; +-- same via ALTER TABLE, with the constraint depending on both the parent's +-- and the inheritance child's row types +create table dp (a int); +create table dc (b int) inherits (dp); +create domain dt_inh as int + check ((row(value)::dp).a > 0 and (row(value, value)::dc).a > 0); +alter table dp alter column a type bigint; +select 1::dt_inh; + dt_inh +-------- + 1 +(1 row) + +select (-1)::dt_inh; -- fail +ERROR: value for domain dt_inh violates check constraint "dt_inh_check" +drop domain dt_inh; +drop table dc; +drop table dp; -- 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 b8f5a639712..f8cf8383667 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -219,6 +219,54 @@ select conname, obj_description(oid, 'pg_constraint') from pg_constraint drop type comptype cascade; +-- check altering columns used by constraints of domains whose base type +-- isn't composite (bug #19724) +create type rt as (i int); +create domain dt as int check ((row(value)::rt).i > 0); +alter type rt alter attribute i type text; -- fail +alter type rt alter attribute i type bigint; +select 1::dt; +select (-1)::dt; -- fail +drop domain dt; +drop type rt; + +-- same for a domain over float8 that uses one field of a two-field type +create type comptype as (r float8, i float8); +create domain silly as float8 check ((row(value, 0)::comptype).r > 0); +alter type comptype alter attribute r type bigint; +select 1.0::silly; +select (-1.0)::silly; -- fail +drop domain silly; +drop type comptype; + +-- rebuilding a constraint for one type it depends on must preserve its +-- dependency on another +create type r1 as (a int); +create type r2 as (b int); +create domain dt_multi as int + check ((row(value)::r1).a > 0 and (row(value)::r2).b > 0); +alter type r1 alter attribute a type bigint; +alter type r2 alter attribute b type text; -- fail +alter type r2 alter attribute b type bigint; +select 1::dt_multi; +select (-1)::dt_multi; -- fail +drop domain dt_multi; +drop type r1; +drop type r2; + +-- same via ALTER TABLE, with the constraint depending on both the parent's +-- and the inheritance child's row types +create table dp (a int); +create table dc (b int) inherits (dp); +create domain dt_inh as int + check ((row(value)::dp).a > 0 and (row(value, value)::dc).a > 0); +alter table dp alter column a type bigint; +select 1::dt_inh; +select (-1)::dt_inh; -- fail +drop domain dt_inh; +drop table dc; +drop table dp; + -- Test domains over arrays of composite -- 2.56.0.rc1.315.gc6ed9934b7-goog
From b8a90f1c6a6c2a2d5730504e4969245cb3b64215 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> 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
