Fix ALTER DOMAIN VALIDATE CONSTRAINT locking
Hi,
While testing "[16a0039dc] Reduce lock level for ALTER DOMAIN ... VALIDATE CONSTRAINT", I found that it doesn't handle concurrent DML properly.
Here is a repro:
1. In session 1, set up and use an advisory lock to block a concurrent insert from session 2:
```
evantest=# create domain d as int;
CREATE DOMAIN
evantest=# create table t (a d);
CREATE TABLE
evantest=# select pg_advisory_lock(8888);
pg_advisory_lock
------------------
(1 row)
```
2. In session 2, run a concurrent insert
```
evantest=# with wait as materialized (select pg_advisory_lock(8888)) insert into t select (-1)::d from wait;
-- block here
```
3. In session 3, alter the domain to add and validate a check constraint
```
evantest=# alter domain d add constraint d_c check (value > 0) not valid;
ALTER DOMAIN
evantest=# alter domain d validate constraint d_c;
ALTER DOMAIN
evantest=# select convalidated from pg_constraint where conname = 'd_c';
convalidated
--------------
t
(1 row)
```
4. In session 1, unlock to let session’s insert continue
```
evantest=# select pg_advisory_unlock(8888);
pg_advisory_unlock
--------------------
t
(1 row)
```
5. Check the result in any session
```
evantest=# select convalidated from pg_constraint where conname = 'd_c';
convalidated
--------------
t
(1 row)
evantest=# select * from t;
a
----
-1
(1 row)
```
As we can see, the constraint is validated, but “-1", which violates the constraint, has been inserted.
The commit message for 16a0039dc says table constraints have the same behavior:
```
Now we should still be able to perform DML operations on table t while
the domain constraint is being validated. The equivalent works
already on table constraints.
```
I don't think that is true for already-running DML. ALTER TABLE ADD CONSTRAINT acquires AccessExclusiveLock, so it waits for a concurrent INSERT on the target table. Here is a similar test with ALTER TABLE:
1. In session 1:
```
evantest=# create table t1 (a int);
CREATE TABLE
evantest=# select pg_advisory_lock(6666);
pg_advisory_lock
------------------
(1 row)
```
2. In session 2
```
evantest=# with wait as materialized (select pg_advisory_lock(6666)) insert into t1 select -1 from wait;
-- block here
```
3. In session 3
```
evantest=# alter table t1 add constraint t_c check (a>0) not valid;
-- block here
```
4. In session 4, unlock
```
evantest=# select pg_advisory_unlock(6666);
pg_advisory_unlock
--------------------
t
(1 row)
```
5. ADD constraint succeeded in session 3:
```
ALTER TABLE
```
6. INSERT also succeeded in session 2:
```
INSERT 0 1
```
7. Then, VALIDATE CONSTRAINT failed, and the constraint’s convalidated remains false:
```
evantest=# alter table t1 validate constraint t_c;
ERROR: check constraint "t_c" of relation "t1" is violated by some row
evantest=# select convalidated from pg_constraint where conname = 't_c';
convalidated
--------------
f
(1 row)
```
So the key difference is that ALTER TABLE ADD CONSTRAINT waits for already-running DML on the target table before adding the NOT VALID constraint, but ALTER DOMAIN
ADD CONSTRAINT does not wait for already-running DML on dependent tables.
The only fix I can see is to use ShareLock again while validating the domain constraint. There is a follow up commit, a99c6b56ffa, and that change looks sound, so maybe we should update the current code instead of reverting 16a0039dc.
With ShareLock restored, rerunning the repro makes session 2's INSERT succeed, but session 3's ALTER DOMAIN VALIDATE CONSTRAINT fails and convalidated remains
false. Now the behavior matches the ALTER TABLE case.
See the attached patch for details. I also added an isolation test that follows the repro above.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v1-0001-Fix-ALTER-DOMAIN-VALIDATE-CONSTRAINT-locking.patchapplication/octet-stream; name=v1-0001-Fix-ALTER-DOMAIN-VALIDATE-CONSTRAINT-locking.patch; x-unix-mode=0644Download+52-5
On Mon, Jun 8, 2026 at 12:54 PM Chao Li <li.evan.chao@gmail.com> wrote:
See the attached patch for details. I also added an isolation test that follows the repro above.
Thanks for the patch! It looks good to me.
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.
Regarding the test, I think it would be safer to also check whether
pg_constraint.convalidated is set correctly. For example:
-----------------------
step s3_validate { ALTER DOMAIN alter_domain_validate_d VALIDATE
CONSTRAINT alter_domain_validate_d_pos; }
+step s3_validated { SELECT convalidated FROM pg_constraint WHERE
conname = 'alter_domain_validate_d_pos'; }
step s3_check { SELECT count(*) FROM alter_domain_validate_t; }-permutation s1_lock s2_insert s3_add s3_validate s1_unlock s3_check
+permutation s1_lock s2_insert s3_add s3_validate s1_unlock s3_validated s3_chec
-----------------------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) §
Regards,
--
Fujii Masao
On Jun 12, 2026, at 19:47, Fujii Masao <masao.fujii@gmail.com> wrote:
On Mon, Jun 8, 2026 at 12:54 PM Chao Li <li.evan.chao@gmail.com> wrote:
See the attached patch for details. I also added an isolation test that follows the repro above.
Thanks for the patch! It looks good to me.
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.Regarding the test, I think it would be safer to also check whether
pg_constraint.convalidated is set correctly. For example:----------------------- step s3_validate { ALTER DOMAIN alter_domain_validate_d VALIDATE CONSTRAINT alter_domain_validate_d_pos; } +step s3_validated { SELECT convalidated FROM pg_constraint WHERE conname = 'alter_domain_validate_d_pos'; } step s3_check { SELECT count(*) FROM alter_domain_validate_t; }-permutation s1_lock s2_insert s3_add s3_validate s1_unlock s3_check +permutation s1_lock s2_insert s3_add s3_validate s1_unlock s3_validated s3_chec -----------------------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) §Regards,
--
Fujii Masao
Hi Fujii-san,
Thanks for picking this up. PFA v2 that addressed your comments.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v2-0001-Fix-ALTER-DOMAIN-VALIDATE-CONSTRAINT-locking.patchapplication/octet-stream; name=v2-0001-Fix-ALTER-DOMAIN-VALIDATE-CONSTRAINT-locking.patch; x-unix-mode=0644Download+62-8
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.
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.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)
On Jun 12, 2026, at 21:02, Álvaro Herrera <alvherre@kurilemu.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/
Import Notes
Reply to msg id not found: aiwCM8TnJOPDUoOC@alvherre.pgsqlReference msg id not found: aiwCM8TnJOPDUoOC@alvherre.pgsql
On Fri, Jun 12, 2026 at 11:02 PM Chao Li <li.evan.chao@gmail.com> wrote:
From a runtime behavior perspective, yes, this patch reverts the behavior change made by 16a0039dc0d1.
Yes.
However:
* 16a0039dc0d1 also refactored validateDomainCheckConstraint() to allow passing in the lock mode, and I think that refactoring is still useful and maybe worth keeping.
I'd prefer to remove that refactoring code, since after the fix there
is no longer any user of the lockmode argument in
validateDomainCheckConstraint().
* 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.
Yes, but fixing that conflict does not seem very difficult, no?
* This patch also adds a test to prevent future changes from making the same mistake.
+1 for adding the test!
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.
Agreed.
Regards,
--
Fujii Masao
On Jun 14, 2026, at 23:52, Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, Jun 12, 2026 at 11:02 PM Chao Li <li.evan.chao@gmail.com> wrote:
From a runtime behavior perspective, yes, this patch reverts the behavior change made by 16a0039dc0d1.
Yes.
However:
* 16a0039dc0d1 also refactored validateDomainCheckConstraint() to allow passing in the lock mode, and I think that refactoring is still useful and maybe worth keeping.
I'd prefer to remove that refactoring code, since after the fix there
is no longer any user of the lockmode argument in
validateDomainCheckConstraint().* 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.
Yes, but fixing that conflict does not seem very difficult, no?
* This patch also adds a test to prevent future changes from making the same mistake.
+1 for adding the test!
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.
Agreed.
Regards,
--
Fujii Masao
Okay, I manually reverted 16a0039dc0d1 with retaining a99c6b56f. I also updated the commit message accordingly. See the attached v3.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v3-0001-Fix-ALTER-DOMAIN-VALIDATE-CONSTRAINT-locking.patchapplication/octet-stream; name=v3-0001-Fix-ALTER-DOMAIN-VALIDATE-CONSTRAINT-locking.patch; x-unix-mode=0644Download+61-18
On Mon, Jun 15, 2026 at 10:11 AM Chao Li <li.evan.chao@gmail.com> wrote:
Okay, I manually reverted 16a0039dc0d1 with retaining a99c6b56f. I also updated the commit message accordingly. See the attached v3.
Thanks for updating the patch! Barring any objections, I will commit it.
Regards,
--
Fujii Masao
On Wed, Jun 17, 2026 at 9:12 AM Fujii Masao <masao.fujii@gmail.com> wrote:
On Mon, Jun 15, 2026 at 10:11 AM Chao Li <li.evan.chao@gmail.com> wrote:
Okay, I manually reverted 16a0039dc0d1 with retaining a99c6b56f. I also updated the commit message accordingly. See the attached v3.
Thanks for updating the patch! Barring any objections, I will commit it.
I've pushed the patch. Thanks!
Regards,
--
Fujii Masao