agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Matheus Alcantara <matheusssilv97@gmail.com>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: pgsql-hackers@lists.postgresql.org
Cc: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Subject: Re: [PATCH v1] Fix for Bug#19724 - ALTER TYPE ... ALTER ATTRIBUTE triggers internal error for base type of domain with check
Date: Tue, 29 Sep 2026 10:54:28 -0300
Message-ID: <DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com> (raw)
In-Reply-To: <CAN4CZFMhvTw7J8kNcU+ZxxWq2oUg+-hiaeVH=sdEt1+rtOK3Pg@mail.gmail.com>
References: <CAH5HC94+4teDZvuWkBiAikcK1pM2DH9W6iCNDoGpHxUdPGTNPw@mail.gmail.com>
<CAJTYsWUwEebLYkSD_+2L6tOp8nL9-TyaLkOYcb1q6viAH=ejNw@mail.gmail.com>
<DLR67P6TO2C8.3SL4J7XR66QBJ@gmail.com>
<CAN4CZFMhvTw7J8kNcU+ZxxWq2oUg+-hiaeVH=sdEt1+rtOK3Pg@mail.gmail.com>
Thank you for reviewing the patch!
On 28/09/26 20:02, Zsolt Parragi wrote:
> Hello
>
> + /*
> + * 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);
>
> That assertion can fire with two concurrent sessions, it should be a
> proper error message, similar to what's inside checkDomainOwner.
>
> See the following isolation test:
>
> setup
> {
> CREATE TYPE ct AS (i int);
> CREATE DOMAIN d AS ct CONSTRAINT d_check CHECK ((VALUE).i > 0);
> CREATE TABLE t2 (x int CONSTRAINT t2_check CHECK ((row(x)::ct).i > 0));
> INSERT INTO t2 VALUES (1);
> }
>
> teardown
> {
> DROP TABLE IF EXISTS t2;
> DROP TYPE IF EXISTS d CASCADE;
> DROP DOMAIN IF EXISTS d_old CASCADE;
> DROP TYPE IF EXISTS ct CASCADE;
> }
>
> session s1
> step a_alter { ALTER TYPE ct ALTER ATTRIBUTE i TYPE bigint; }
>
> session s2
> step b_begin { BEGIN; SELECT count(*) FROM t2; }
> step b_commit { COMMIT; }
>
> session s3
> step c_swap { ALTER DOMAIN d RENAME TO d_old; CREATE TYPE d AS (z int); }
>
> permutation b_begin a_alter c_swap b_commit
>
>
Good catch. I've changed to use ereport like checkDomainOwner().
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.
> + if (!con->skip_validation)
> + tab->domain_constraints =
> + lappend_oid(tab->domain_constraints,
> + constrAddr.objectId);
>
> This can be uninitialized, AlterDomainAddConstraint doesn't guarantee
> a write. I think this could use both a Assert(con->contype ==
> CONSTR_CHECK); and initalizating constrAddr to InvalidObjectAddress.
Fixed.
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
From a1296adad6923059274fad35de53200b5b1372c2 Mon Sep 17 00:00:00 2001
From: Nitin Motiani <nitinmotiani@google.com>
Date: Mon, 28 Sep 2026 12:49:33 +0000
Subject: [PATCH v3 1/3] Fix ALTER TYPE ... ALTER ATTRIBUTE on types used in
domain constraints.
Commit af20e2d72 updated ALTER TABLE / TYPE to rebuild domain constraints
when an attribute of a composite type is altered. However, it assumed that
the domain's base type was always the composite type being altered, calling
get_typ_typrelid(getBaseType(con->contypid)). If the domain was defined
over a scalar type (such as int or float8) whose CHECK expression referenced
the composite type, get_typ_typrelid() returned InvalidOid, triggering
an internal "could not identify relation associated with constraint" error.
Fix by attaching the deferred domain constraint rebuild command to the
table being altered (tab->relid) rather than attempting to derive a relation
OID from the domain's base type. Domains do not have pg_class relations of
their own, and the rebuild command (AlterDomainStmt) is self-contained.
Reported-by: Alexander Lakhin
Bug: #19724
---
src/backend/commands/tablecmds.c | 10 +++---
src/test/regress/expected/domain.out | 50 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 33 ++++++++++++++++++
3 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0274d892f2e..c8bc193a2ab 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16141,10 +16141,12 @@ 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. Domains don't have their own
+ * relations, so attach the rebuild step to the table being
+ * altered.
+ */
+ 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..de60a90c045 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -432,6 +432,56 @@ select conname, obj_description(oid, 'pg_constraint') from pg_constraint
drop type comptype cascade;
NOTICE: drop cascades to type dcomptype
+-- regression tests for bug #19724
+-- test scenario from bug report, plus failure when changing int to text
+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 cascade;
+-- test silly example from Tom Lane's 2017 email (domain over float8)
+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 cascade;
+-- test domain constraint referencing multiple composite types
+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 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 cascade;
+drop type r2 cascade;
-- 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..1240f9422bd 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -219,6 +219,39 @@ select conname, obj_description(oid, 'pg_constraint') from pg_constraint
drop type comptype cascade;
+-- regression tests for bug #19724
+
+-- test scenario from bug report, plus failure when changing int to text
+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 cascade;
+
+-- test silly example from Tom Lane's 2017 email (domain over float8)
+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 cascade;
+
+-- test domain constraint referencing multiple composite types
+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 bigint;
+select 1::dt_multi;
+select (-1)::dt_multi; -- fail
+drop domain dt_multi;
+drop type r1 cascade;
+drop type r2 cascade;
+
-- Test domains over arrays of composite
--
2.50.1 (Apple Git-155)
From 1cf5d63cd2ebc1f0e4826794ec308382b1e9952c Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Mon, 28 Sep 2026 15:05:36 -0300
Subject: [PATCH v3 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 c8bc193a2ab..d93c2e0d452 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 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)
From 06a359a989b3124a3acca50d22f878ffee72e777 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Mon, 28 Sep 2026 15:18:56 -0300
Subject: [PATCH v3 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 d93c2e0d452..1a5bb53e1f3 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 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)
Attachments:
[text/plain] v3-0001-Fix-ALTER-TYPE-.-ALTER-ATTRIBUTE-on-types-used-in.patch (5.7K, ../DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com/2-v3-0001-Fix-ALTER-TYPE-.-ALTER-ATTRIBUTE-on-types-used-in.patch)
download | inline diff:
From a1296adad6923059274fad35de53200b5b1372c2 Mon Sep 17 00:00:00 2001
From: Nitin Motiani <nitinmotiani@google.com>
Date: Mon, 28 Sep 2026 12:49:33 +0000
Subject: [PATCH v3 1/3] Fix ALTER TYPE ... ALTER ATTRIBUTE on types used in
domain constraints.
Commit af20e2d72 updated ALTER TABLE / TYPE to rebuild domain constraints
when an attribute of a composite type is altered. However, it assumed that
the domain's base type was always the composite type being altered, calling
get_typ_typrelid(getBaseType(con->contypid)). If the domain was defined
over a scalar type (such as int or float8) whose CHECK expression referenced
the composite type, get_typ_typrelid() returned InvalidOid, triggering
an internal "could not identify relation associated with constraint" error.
Fix by attaching the deferred domain constraint rebuild command to the
table being altered (tab->relid) rather than attempting to derive a relation
OID from the domain's base type. Domains do not have pg_class relations of
their own, and the rebuild command (AlterDomainStmt) is self-contained.
Reported-by: Alexander Lakhin
Bug: #19724
---
src/backend/commands/tablecmds.c | 10 +++---
src/test/regress/expected/domain.out | 50 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 33 ++++++++++++++++++
3 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0274d892f2e..c8bc193a2ab 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16141,10 +16141,12 @@ 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. Domains don't have their own
+ * relations, so attach the rebuild step to the table being
+ * altered.
+ */
+ 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..de60a90c045 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -432,6 +432,56 @@ select conname, obj_description(oid, 'pg_constraint') from pg_constraint
drop type comptype cascade;
NOTICE: drop cascades to type dcomptype
+-- regression tests for bug #19724
+-- test scenario from bug report, plus failure when changing int to text
+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 cascade;
+-- test silly example from Tom Lane's 2017 email (domain over float8)
+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 cascade;
+-- test domain constraint referencing multiple composite types
+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 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 cascade;
+drop type r2 cascade;
-- 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..1240f9422bd 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -219,6 +219,39 @@ select conname, obj_description(oid, 'pg_constraint') from pg_constraint
drop type comptype cascade;
+-- regression tests for bug #19724
+
+-- test scenario from bug report, plus failure when changing int to text
+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 cascade;
+
+-- test silly example from Tom Lane's 2017 email (domain over float8)
+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 cascade;
+
+-- test domain constraint referencing multiple composite types
+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 bigint;
+select 1::dt_multi;
+select (-1)::dt_multi; -- fail
+drop domain dt_multi;
+drop type r1 cascade;
+drop type r2 cascade;
+
-- Test domains over arrays of composite
--
2.50.1 (Apple Git-155)
[text/plain] v3-0002-Validate-re-added-domain-constraints-after-ALTER-.patch (14.2K, ../DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com/3-v3-0002-Validate-re-added-domain-constraints-after-ALTER-.patch)
download | inline diff:
From 1cf5d63cd2ebc1f0e4826794ec308382b1e9952c Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Mon, 28 Sep 2026 15:05:36 -0300
Subject: [PATCH v3 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 c8bc193a2ab..d93c2e0d452 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 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)
[text/plain] v3-0003-Don-t-require-ownership-when-rebuilding-constrain.patch (9.1K, ../DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com/4-v3-0003-Don-t-require-ownership-when-rebuilding-constrain.patch)
download | inline diff:
From 06a359a989b3124a3acca50d22f878ffee72e777 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Mon, 28 Sep 2026 15:18:56 -0300
Subject: [PATCH v3 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 d93c2e0d452..1a5bb53e1f3 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 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)
view thread (11+ messages) latest in thread
Message-ID: <DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com>
Permalink: ../DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com/
Also on: postgresql.org/message-id/DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: matheusssilv97@gmail.com, zsolt.parragi@percona.com, pgsql-hackers@lists.postgresql.org, ayushtiwari.slg01@gmail.com
Subject: Re: [PATCH v1] Fix for Bug#19724 - ALTER TYPE ... ALTER ATTRIBUTE triggers internal error for base type of domain with check
In-Reply-To: <DLRUNGYX33TS.2M17VTR2OBXPC@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox