| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Peter Eisentraut <peter(at)eisentraut(dot)org>, jian he <jian(dot)universality(at)gmail(dot)com> |
| Subject: | Re: Fix ALTER DOMAIN VALIDATE CONSTRAINT locking |
| Date: | 2026-06-12 14:01:44 |
| Message-ID: | 6F7DCFA0-E511-4718-8781-5ABE3D599A1B@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Jun 12, 2026, at 21:02, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> Hello,
>
> On 2026-Jun-12, Fujii Masao wrote:
>
>> I just have a few minor review comments.
>>
>> * The lockmode is used for relations using the domain. It should be
>> * ShareLock when adding a new constraint to domain. It can be
>> * ShareUpdateExclusiveLock when validating an existing constraint.
>>
>> This comment in validateDomainCheckConstraint() still references
>> ShareUpdateExclusiveLock, so it seems to need updating.
>
> I may be missing something, but doesn't changing the
> ShareUpdateExclusive lock to ShareLock means essentially reverting
> 16a0039dc0d1? I mean, the code was previously using ShareLock for both
> uses of validateDomainCheckConstraint(); and what that commit did was
> change AlterDomainValidateConstraint() to use ShareUpdateExclusiveLock,
> while AlterDomainAddConstraint() retained the stronger ShareLock level.
> If we now change AlterDomainValidateConstraint() back to ShareLock, then
> the aforementioned commit has no effect whatsoever.
From a runtime behavior perspective, yes, this patch reverts the behavior change made by 16a0039dc0d1.
However:
* 16a0039dc0d1 also refactored validateDomainCheckConstraint() to allow passing in the lock mode, and I think that refactoring is still useful and maybe worth keeping.
* A follow-up commit, a99c6b56f, made validating an already-validated constraint a no-op. A direct revert of 16a0039dc0d1 would conflict with later changes around this code.
* This patch also adds a test to prevent future changes from making the same mistake.
>
>> After the fix is committed, we should probably also ask Bruce to update
>> the following v19 release note item?:
>>
>> Reduce lock level of ALTER DOMAIN ... VALIDATE CONSTRAINT to match
>> ALTER TABLE ... VALIDATE CONSTRAINT (Jian He) §
>
> If we make this change, then the release note item should be removed
> entirely, ISTM.
>
True. Once this patch is pushed, this item should be removed from the release note.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-06-12 14:22:37 | Re: Use \if/\endif to remove non-libxml2 expected output in regression tests |
| Previous Message | Tom Lane | 2026-06-12 13:50:01 | Re: Why our Valgrind reports suck |