| From: | Nitin Motiani <nitinmotiani(at)google(dot)com> |
|---|---|
| To: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
| Cc: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
| Subject: | Re: [PATCH v1] Fix for Bug#19724 - ALTER TYPE ... ALTER ATTRIBUTE triggers internal error for base type of domain with check |
| Date: | 2026-10-03 10:20:38 |
| Message-ID: | CAH5HC945XpFgKhZK9vQug5-A=_-dWRt2_LzOp9unA4JEAA941Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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(at)gmail(dot)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
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0003-Don-t-require-ownership-when-rebuilding-constrain.patch | application/x-patch | 9.1 KB |
| v4-0001-Fix-ALTER-.-TYPE-failure-with-dependent-domain-co.patch | application/x-patch | 7.8 KB |
| v4-0002-Validate-re-added-domain-constraints-after-ALTER-.patch | application/x-patch | 14.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hannu Krosing | 2026-10-03 11:02:47 | [PATCH] Refactor pgbench to make future improvements easier |
| Previous Message | Matheus Alcantara | 2026-10-03 09:38:37 | Re: postgres_fdw: transaction mode inheritance corner cases |