From bf78ab6a32c5e27572588cfa5867ed4e6f2049b6 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Mon, 28 Sep 2026 15:18:56 -0300 Subject: [PATCH v2 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 | 18 +++++++++---- src/test/regress/expected/domain.out | 38 ++++++++++++++++++++++++++++ src/test/regress/sql/domain.sql | 31 +++++++++++++++++++++++ 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 10c7a3a5f8e..a6c1d43cc0e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5557,7 +5557,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..69cdfd87a79 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -3004,8 +3004,16 @@ 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) + Assert(typTup->typtype == TYPTYPE_DOMAIN); + else + checkDomainOwner(tup); if (!IsA(newConstraint, Constraint)) elog(ERROR, "unrecognized node type: %d", @@ -3034,9 +3042,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 14f2c928700..e49c5dacab3 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -581,6 +581,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 b6e452d6ebb..e914b6913ee 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -315,6 +315,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.50.1 (Apple Git-155)