make VALIDATE domain constraint lock on related relations as ShareUpdateExclusiveLock

Started by jian he8 months ago4 messages
#1jian he
jian.universality@gmail.com
1 attachment(s)

hi.

We can still perform DML operations on a table while validating its
check constraint.
Similarly, it should be fine to do DML while validating domain constraints?
but currently, it's not allowed for domain constraints.

The attached patch addresses this problem.
so, with the patch, the following is allowed:

create domain d1 as int;
create table t(a d1);
alter domain d1 add constraint cc10 check(value > 10) not valid;

begin;
alter domain d1 validate constraint cc10;

--another session
INSERT INTO T SELECT 8;

Attachments:

v1-0001-reduce-lock-level-when-ALTER-DOMAIN.VALIDATE-CONSTRAINT.patchtext/x-patch; charset=US-ASCII; name=v1-0001-reduce-lock-level-when-ALTER-DOMAIN.VALIDATE-CONSTRAINT.patchDownload
From 4e72f52169dad4c0eaaaf77f697c0d2c70c2db5d Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Tue, 13 May 2025 11:04:37 +0800
Subject: [PATCH v1 1/1] reduce lock level when ALTER DOMAIN...VALIDATE
 CONSTRAINT

ON TABLE LEVEL:
create table t2(a int);
alter table t2 add constraint cc10 check(a > 10) not valid;

begin;
alter table t2 validate constraint cc10;

We can still perform DML operations on table t2 while validating its check constraint.
Currently, concurrent DML is not allowed on a domain while its constraint is being validated.
But it should be fine.

so we can reduce ALTER DOMAIN VALIDATE CONSTRAINT to ShareUpdateExclusiveLock
from ShareLock for the relations we are going to validate.

discussion: https://postgr.es/m/
---
 src/backend/commands/typecmds.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 45ae7472ab5..cd92ba8b97b 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -126,7 +126,7 @@ 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 validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmode);
 static void validateDomainNotNullConstraint(Oid domainoid);
 static List *get_rels_with_domain(Oid domainOid, LOCKMODE lockmode);
 static void checkEnumOwner(HeapTuple tup);
@@ -2978,7 +2978,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
 		 * to.
 		 */
 		if (!constr->skip_validation)
-			validateDomainCheckConstraint(domainoid, ccbin);
+			validateDomainCheckConstraint(domainoid, ccbin, ShareLock);
 
 		/*
 		 * We must send out an sinval message for the domain, to ensure that
@@ -3090,7 +3090,12 @@ AlterDomainValidateConstraint(List *names, const char *constrName)
 	val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin);
 	conbin = TextDatumGetCString(val);
 
-	validateDomainCheckConstraint(domainoid, conbin);
+	/*
+	 * lock related relations with ShareUpdateExclusiveLock is ok, since not
+	 * valid constraints will still be enforced against subsequent inserts or
+	 * updates.
+	*/
+	validateDomainCheckConstraint(domainoid, conbin, ShareUpdateExclusiveLock);
 
 	/*
 	 * Now update the catalog, while we have the door open.
@@ -3183,9 +3188,14 @@ validateDomainNotNullConstraint(Oid domainoid)
 /*
  * Verify that all columns currently using the domain satisfy the given check
  * constraint expression.
+ *
+ * It is used to validate existing constraints and to add newly created check
+ * constraints to a domain.
+ * The lockmode is ShareLock when add a new constraint to domain, it can be
+ * ShareUpdateExclusiveLock when validating existing constraint.
  */
 static void
-validateDomainCheckConstraint(Oid domainoid, const char *ccbin)
+validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmode)
 {
 	Expr	   *expr = (Expr *) stringToNode(ccbin);
 	List	   *rels;
@@ -3202,9 +3212,7 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin)
 	exprstate = ExecPrepareExpr(expr, estate);
 
 	/* Fetch relation list with attributes based on this domain */
-	/* ShareLock is sufficient to prevent concurrent data changes */
-
-	rels = get_rels_with_domain(domainoid, ShareLock);
+	rels = get_rels_with_domain(domainoid, lockmode);
 
 	foreach(rt, rels)
 	{
-- 
2.34.1

#2Dilip Kumar
dilipbalaut@gmail.com
In reply to: jian he (#1)
Re: make VALIDATE domain constraint lock on related relations as ShareUpdateExclusiveLock

On Tue, May 13, 2025 at 8:57 AM jian he <jian.universality@gmail.com> wrote:

hi.

We can still perform DML operations on a table while validating its
check constraint.
Similarly, it should be fine to do DML while validating domain constraints?
but currently, it's not allowed for domain constraints.

The attached patch addresses this problem.

This makes sense, and the patch also looks good to me.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

#3wenhui qiu
qiuwenhuifx@gmail.com
In reply to: Dilip Kumar (#2)
Re: make VALIDATE domain constraint lock on related relations as ShareUpdateExclusiveLock

HI
Thanks for the patch! and overall this is a valuable improvement in
terms of concurrency and consistency with table-level behavior.The
refactoring to pass LOCK MODE into validateDomainCheckConstraint() improves
flexibility and clarity. and the patch also looks good to me.

Regards

On Tue, May 13, 2025 at 2:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

Show quoted text

On Tue, May 13, 2025 at 8:57 AM jian he <jian.universality@gmail.com>
wrote:

hi.

We can still perform DML operations on a table while validating its
check constraint.
Similarly, it should be fine to do DML while validating domain

constraints?

but currently, it's not allowed for domain constraints.

The attached patch addresses this problem.

This makes sense, and the patch also looks good to me.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

#4Peter Eisentraut
peter@eisentraut.org
In reply to: wenhui qiu (#3)
Re: make VALIDATE domain constraint lock on related relations as ShareUpdateExclusiveLock

Committed.

Note that the proposed commit message had an example using ALTER TABLE,
which was not right. I used an example from your email using ALTER
DOMAIN instead.

Show quoted text

On 13.05.25 11:34, wenhui qiu wrote:

HI
   Thanks for the patch! and overall this is a valuable improvement in
terms of concurrency and consistency with table-level behavior.The
refactoring to pass LOCK MODE into validateDomainCheckConstraint()
improves flexibility and clarity. and the patch also looks good to me.

Regards

On Tue, May 13, 2025 at 2:23 PM Dilip Kumar <dilipbalaut@gmail.com
<mailto:dilipbalaut@gmail.com>> wrote:

On Tue, May 13, 2025 at 8:57 AM jian he <jian.universality@gmail.com
<mailto:jian.universality@gmail.com>> wrote:

hi.

We can still perform DML operations on a table while validating its
check constraint.
Similarly, it should be fine to do DML while validating domain

constraints?

but currently, it's not allowed for domain constraints.

The attached patch addresses this problem.

This makes sense, and the patch also looks good to me.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com <http://www.enterprisedb.com&gt;