[PATCH v1] Fix for Bug#19724 - ALTER TYPE ... ALTER ATTRIBUTE triggers internal error for base type of domain with check

Hi,

On Mon, 28 Sept 2026 at 18:59, Nitin Motiani <nitinmotiani@google.com>
wrote:

The following bug was reported in [1]

```
CREATE TYPE rt AS (i int);
CREATE DOMAIN dt AS int CHECK ((ROW(value)::rt).i > 0);
ALTER TYPE rt ALTER ATTRIBUTE i TYPE bigint;

triggers an internal
ERROR: XX000: could not identify relation associated with constraint

16390

LOCATION: ATPostAlterTypeCleanup, tablecmds.c:16147

Reproduced starting from af20e2d72.

```

I am not sure how common this scenario is but I investigated the
history of the commit af20e2d72. LLM pointed me to [2] from 2017.

The thread mentions a similar case in the email but it isn't covered
in the test cases.

```
regression=# create type comptype as (r float8, i float8);
CREATE TYPE
regression=# create domain silly as float8 check
((row(value,0)::comptype).r > 0);
CREATE DOMAIN
regression=# alter type comptype alter attribute r type varchar;
ERROR: cache lookup failed for relation 0
```

Therefore I am attaching a patch file with a proposed fix. The issue
stems from the fact that for a domain constraint, we currently look
for the domain's base type and the corresponding relid. But if the
domain is over a primitive type like int, there is no relid. And
therefore it fails.

My understanding of code is that this relid is only being used in
ATPostAlterTypeParse to unqueue the entry corresponding to the type
being altered.

So in this patch instead of getting the relid from the domain, we use
the relid of the type being altered.

I tested changing int to bigint and text to ensure it passes in the
first case and fails in the second.

Please take a look and let me know what you think.

Thanks for working on this. Switching to tab->relid looks right to me,
since a domain constraint doesn't belong to any relation anyway.

I think there's still a problem when a table that uses the domain gets
rewritten by the same ALTER, though. The re-added constraint is
validated right away in phase 2, before phase 3 has rewritten those
tables, so their old rows are read with the new tuple descriptor. With
the patch this crashes for me:

create table dtab (i int);
create domain dtext as text check ((row(1)::dtab).i > 0 and md5(value) <>
'');
create table dtab_child (t dtext) inherits (dtab);
insert into dtab_child values (1, repeat('x', 200));
alter table dtab alter column i type bigint;

On master it just fails with the elog. (FWIW master, and 14 too as far
as I checked, can already crash like this with a domain over the
parent's rowtype stored in the child, so it's not entirely new.)

Maybe the constraint could be re-added as NOT VALID in phase 2, and then
validated once phase 3 is done, e.g. with AlterDomainValidateConstraint()
next to the FK checks at the end of ATRewriteTables()?

Also, I believe the test cases can be trimmed.

For the tests, all three cases take the same path and none of them
stores the domain in a table. One test for the reported case plus one
like the above might be enough.

Regards,
Ayush

[1]: /messages/by-id/19724-58468097b5b17d10@postgresql.org
/messages/by-id/19724-58468097b5b17d10@postgresql.org
[2]: /messages/by-id/30656.1509128130@sss.pgh.pa.us
/messages/by-id/30656.1509128130@sss.pgh.pa.us

Hi Nitin,

I reviewed and tested v1. The fix looks right to me. For a domain
constraint, the relation OID in ATPostAlterTypeCleanup() only decides
whether another relation gets locked and which work-queue entry gets
the AT_ReAddDomainConstraint command. The AlterDomainStmt names the
domain itself, so using tab->relid is fine.

Testing (macOS arm64, meson debug build with assertions, master at
ad36e3608c plus v1): v1 applies cleanly with git am, and the
regression tests pass. I also ran these cases on master and on the
patched build. Master fails on all of them with "could not identify
relation associated with constraint". With v1 the type change goes
through and the constraint is rebuilt:

- ALTER TABLE ... ALTER COLUMN ... TYPE on a table whose row type is
used in the check of a domain over int
- a domain over int[], and a domain over a domain over int
- a typed table, with ALTER TYPE ... CASCADE
- one domain check that uses both a parent's and a child's row type,
with ALTER TABLE on the parent
- a table column of the domain type with data in it (the constraint
is revalidated)
- a comment on the constraint (kept) and a NOT VALID constraint
(stays NOT VALID)

Where I checked, the rebuilt constraint still rejects bad values.

Comments:

1. The bug isn't specific to ALTER TYPE. ALTER TABLE ... ALTER
COLUMN ... TYPE hits it too, as in the first case above. I'd add
a test for that and mention it in the commit message.

2. The commit message says the domain is "defined over a scalar
type". Arrays and domains over domains are affected too, so "a
domain whose base type isn't composite" would be more accurate.

3. In the tests, I'd describe the float8 case instead of referring to
"Tom Lane's 2017 email".

4. One side effect: for a domain over a different composite type, as
in

CREATE DOMAIN dd AS r2 CHECK ((ROW((VALUE).b)::r1).a > 0);
ALTER TYPE r1 ALTER ATTRIBUTE a TYPE bigint;

master takes an AccessExclusiveLock on r2 and queues the rebuild
there, while v1 does neither. I think that's fine, since the
rebuilt constraint belongs to the domain and doesn't touch r2.

5. af20e2d72 went into v11, so every supported branch has this bug.
v1, tests included, applies cleanly to REL_14_STABLE through
REL_19_STABLE, so +1 for back-patching.

I couldn't find this in the open commitfest (PG20-3). Could you add
it? I'm happy to be listed as a reviewer.

Regards,
Rahul Yadav

Thanks Rahul.

I've created the commitfest entry here
https://commitfest.postgresql.org/patch/7356/.

Regards,
Nitin Motiani
Google

Hi,

I've reviewed and tested Nitin's v1 as well, and it works as expected.
The reported case and the variants I tried go through, and the rebuilt
constraint is still enforced. For a domain constraint the relid only
decides which relation gets locked and which work queue entry gets the
AT_ReAddDomainConstraint command, so tab->relid seems right to me.

On Mon Sep 28, 2026 at 11:31 AM -03, Ayush Tiwari wrote:

Thanks for working on this. Switching to tab->relid looks right to me,
since a domain constraint doesn't belong to any relation anyway.

I think there's still a problem when a table that uses the domain gets
rewritten by the same ALTER, though. The re-added constraint is
validated right away in phase 2, before phase 3 has rewritten those
tables, so their old rows are read with the new tuple descriptor. With
the patch this crashes for me:

Yes, I found the same problem while testing v1, plus a second one.
Both date back to af20e2d72 and can already be hit on master with a
domain over a composite type, but with v1 they are reachable for any
domain whose check expression references a composite type.

1. Re-added domain constraints are validated before the rewrite.

As you describe, AT_ReAddDomainConstraint calls
AlterDomainAddConstraint(), which validates the constraint immediately,
while tables using the domain may still be pending a rewrite. A simple
way to see it:

create function show(int) returns bool language plpgsql as
$$ begin raise notice 'domain check sees value %', $1; return true; end $$;
create table t (c int);
create domain dt as int check (show(value) and (null::t).c is null);
alter table t add column d dt;
insert into t values (1, 5), (2, 7);
alter table t alter column c type bigint;
NOTICE: domain check sees value 0
NOTICE: domain check sees value 700

Depending on the data this gives garbage values, spurious "contains
values that violate the new constraint" errors, errors like "type with
OID 4294967295 does not exist" (domain over composite, on master), or
a crash like in your example.

2. Rebuilding the constraint requires ownership of the domain.

AlterDomainAddConstraint() calls checkDomainOwner(), so if another
user's domain has a constraint that depends on your type, you can't
alter your own type:

-- as alice
create type rt as (i int);
-- as bob
create domain dt as int check ((row(value)::rt).i > 0);
-- as alice
alter type rt alter attribute i type bigint;
ERROR: must be owner of type dt

Since types grant USAGE to PUBLIC by default, any user can create such
a dependency. The matching drop is done without permission checks, and
rebuilding another user's table CHECK constraint doesn't check
ownership either. A related case: if the constraint has a comment, it
is restored via CommentObject(), which also requires ownership. That
affects table constraints too, even without domains:

-- as bob
create table tt (x int constraint k check ((row(x)::rt).i > 0));
comment on constraint k on tt is 'hi';
-- as alice
alter type rt alter attribute i type bigint;
ERROR: must be owner of relation tt

create table dtab (i int);
create domain dtext as text check ((row(1)::dtab).i > 0 and md5(value) <>
'');
create table dtab_child (t dtext) inherits (dtab);
insert into dtab_child values (1, repeat('x', 200));
alter table dtab alter column i type bigint;

On master it just fails with the elog. (FWIW master, and 14 too as far
as I checked, can already crash like this with a domain over the
parent's rowtype stored in the child, so it's not entirely new.)

Maybe the constraint could be re-added as NOT VALID in phase 2, and then
validated once phase 3 is done, e.g. with AlterDomainValidateConstraint()
next to the FK checks at the end of ATRewriteTables()?

I considered that, but I think it's simpler to skip validation when
re-adding, remember the new constraint's OID in the work queue entry,
and call validateDomainCheckConstraint() on it once all tables have
been rewritten. My reasons:

- AlterDomainValidateConstraint() also calls checkDomainOwner(), so it
has issue 2 as well. Unlike AlterDomainAddConstraint() it has no
is_readd flag, so we'd need to add a new parameter to it too.

- It looks up the domain and the constraint by name. Using the OID
avoids resolving the names again in phase 3; ATPostAlterTypeCleanup()
already works from OIDs for similar reasons.

- We'd still need to remember which constraints were valid before the
rebuild, so that a NOT VALID constraint isn't validated. That's the
same bookkeeping as queuing the OID.

- The FK validation loop skips relations without storage. For ALTER TYPE
on a standalone composite type the only work queue entry is the type
itself, so validating there would silently skip it. For example, with
a stored value of 40000, changing an attribute from int to smallint
must still fail with "smallint out of range". The new loop on the
attached 0002 patch runs over the whole work queue for this reason.

One more thing I noticed, not addressed by these patches: if a domain
check uses ROW(value)::t and a column of the domain is later added to
t, the deparsed constraint becomes ROW(VALUE, NULL)::t. Re-parsing
that coerces the NULL to the domain itself, so the rebuilt constraint
refers to its own domain and fails with "stack depth limit exceeded".
The same happens with a hand-written ALTER DOMAIN ... ADD CONSTRAINT
using ROW(value, null)::t, so it's not specific to this code path,
which is why the tests use (null::t).c instead.

Attached are:

- v2-0001: Nitin's v1, unchanged.

- v2-0002: validate re-added domain constraints after the rewrites
(issue 1).

- v2-0003: don't require ownership when rebuilding constraints and their
comments (issue 2).

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

Hi,

On Tue, 29 Sept 2026 at 00:15, Matheus Alcantara
<matheusssilv97@gmail.com> wrote:

Yes, I found the same problem while testing v1, plus a second one.
Both date back to af20e2d72 and can already be hit on master with a
domain over a composite type, but with v1 they are reachable for any
domain whose check expression references a composite type.

1. Re-added domain constraints are validated before the rewrite.

As you describe, AT_ReAddDomainConstraint calls
AlterDomainAddConstraint(), which validates the constraint immediately,
while tables using the domain may still be pending a rewrite. A simple
way to see it:

create function show(int) returns bool language plpgsql as
$$ begin raise notice 'domain check sees value %', $1; return true; end $$;
create table t (c int);
create domain dt as int check (show(value) and (null::t).c is null);
alter table t add column d dt;
insert into t values (1, 5), (2, 7);
alter table t alter column c type bigint;
NOTICE: domain check sees value 0
NOTICE: domain check sees value 700

Depending on the data this gives garbage values, spurious "contains
values that violate the new constraint" errors, errors like "type with
OID 4294967295 does not exist" (domain over composite, on master), or
a crash like in your example.

On master it just fails with the elog. (FWIW master, and 14 too as far
as I checked, can already crash like this with a domain over the
parent's rowtype stored in the child, so it's not entirely new.)

Maybe the constraint could be re-added as NOT VALID in phase 2, and then
validated once phase 3 is done, e.g. with AlterDomainValidateConstraint()
next to the FK checks at the end of ATRewriteTables()?

I considered that, but I think it's simpler to skip validation when
re-adding, remember the new constraint's OID in the work queue entry,
and call validateDomainCheckConstraint() on it once all tables have
been rewritten. My reasons:

- AlterDomainValidateConstraint() also calls checkDomainOwner(), so it
has issue 2 as well. Unlike AlterDomainAddConstraint() it has no
is_readd flag, so we'd need to add a new parameter to it too.

- It looks up the domain and the constraint by name. Using the OID
avoids resolving the names again in phase 3; ATPostAlterTypeCleanup()
already works from OIDs for similar reasons.

- We'd still need to remember which constraints were valid before the
rebuild, so that a NOT VALID constraint isn't validated. That's the
same bookkeeping as queuing the OID.

- The FK validation loop skips relations without storage. For ALTER TYPE
on a standalone composite type the only work queue entry is the type
itself, so validating there would silently skip it. For example, with
a stored value of 40000, changing an attribute from int to smallint
must still fail with "smallint out of range". The new loop on the
attached 0002 patch runs over the whole work queue for this reason.

One more thing I noticed, not addressed by these patches: if a domain
check uses ROW(value)::t and a column of the domain is later added to
t, the deparsed constraint becomes ROW(VALUE, NULL)::t. Re-parsing
that coerces the NULL to the domain itself, so the rebuilt constraint
refers to its own domain and fails with "stack depth limit exceeded".
The same happens with a hand-written ALTER DOMAIN ... ADD CONSTRAINT
using ROW(value, null)::t, so it's not specific to this code path,
which is why the tests use (null::t).c instead.

Attached are:

- v2-0001: Nitin's v1, unchanged.

- v2-0002: validate re-added domain constraints after the rewrites
(issue 1).

Thanks, the split looks right to me.

On 0002, remembering the new constraint OID and calling
validateDomainCheckConstraint() directly is better than what I suggested.
Using the OID avoids resolving the domain and constraint by name again in
phase 3, and the standalone-composite case needs the new loop not to skip
relations without storage. One small thing: the new loop doesn't
CommandCounterIncrement() between constraints. Probably fine today, but
the FK loop and afterStmts do. And I think 0001 and 0002 can be clubbed
together (though that can be done whilst committing)

I've only looked closely at 0001 and 0002 so far, which fix the reported
case for me across branches. I'll come back on 0003.

Regards,
Ayush

On 28/09/26 17:40, Ayush Tiwari wrote:

Attached are:

- v2-0001: Nitin's v1, unchanged.

- v2-0002: validate re-added domain constraints after the rewrites
(issue 1).

Thanks, the split looks right to me.

On 0002, remembering the new constraint OID and calling
validateDomainCheckConstraint() directly is better than what I suggested.
Using the OID avoids resolving the domain and constraint by name again in
phase 3, and the standalone-composite case needs the new loop not to skip
relations without storage. One small thing: the new loop doesn't
CommandCounterIncrement() between constraints. Probably fine today, but
the FK loop and afterStmts do. And I think 0001 and 0002 can be clubbed
together (though that can be done whilst committing)

Thank you for checking the patches.

I don't think that the FK loop call CommandCounterIncrement() or I'm
missing something? Also I think that afterStmts call it because it use
ProcessUtilityForAlterTable, so I don't think that it is required for
the new domain constraints loop, but I might be wrong.

I'm not sure if these two patches should be squashed into a single
one. I see these both issues as separated issues, although the fix on
0001 enable the second issue to happen more easily.

I've only looked closely at 0001 and 0002 so far, which fix the reported
case for me across branches. I'll come back on 0003.

Thank you!

--
Matheus Alcantara
EDB: https://www.enterprisedb.com

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

+				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.

Thanks everyone for the feedback. I'll work on incorporating that.

I'm also looking into 0002 and 0003 from Matheus. From a quick scan,
it looks like those issues have existed since earlier, even in the
cases involving domains over composite types. So perhaps it's better
to have them as separate patches. But I'll look in more detail to see
if 0001 and 0002 should be squashed together.

Regards,
Nitin Motiani
Google

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

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
<matheusssilv97@gmail.com> 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