On Wed, May 21, 2025 at 2:49 PM jian he <[email protected]> wrote:
>
> hi.
>
> Looking at AlterDomainValidateConstraint, it seems currently, ALTER
> DOMAIN VALIDATE
> CONSTRAINT will re-validate a VALID constraint, which
> would just waste cycles.
> Ideally, this operation should be a no-op.
>
> The attached patch addresses this by making ALTER DOMAIN VALIDATE CONSTRAINT a
> no-op in such cases.
>
> ALTER TABLE VALIDATE CONSTRAINT is already a no-op when the constraint is 
> VALID.

hi.

simple rebase.
From f5e7bc9eeb93e29e47fdbbd3e1beef681b3a792f Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Fri, 10 Oct 2025 22:39:32 +0800
Subject: [PATCH v2 1/1] make ALTER DOMAIN VALIDATE CONSTRAINT no-op when
 constraint is validated

Looking at AlterDomainValidateConstraint, it seems currently, ALTER DOMAIN
VALIDATE CONSTRAINT will re-validate a constraint that has already been
validated, which would just waste cycles.  Ideally, this operation should be a
no-op when the constraint is already validated.

This also align with ATExecValidateConstraint.

discussion: https://postgr.es/m/CACJufxG=-dv9fpjhqka9c-wgz2ddowoxsp-x-0k_g7r-dga...@mail.gmail.com
---
 src/backend/commands/typecmds.c | 41 ++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index c6de04819f1..33267ff7ab5 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -3044,7 +3044,7 @@ AlterDomainValidateConstraint(List *names, const char *constrName)
 	HeapTuple	tuple;
 	HeapTuple	copyTuple;
 	ScanKeyData skey[3];
-	ObjectAddress address;
+	ObjectAddress address = InvalidObjectAddress;
 
 	/* Make a TypeName so we can use standard type lookup machinery */
 	typename = makeTypeNameFromNameList(names);
@@ -3095,29 +3095,32 @@ AlterDomainValidateConstraint(List *names, const char *constrName)
 				 errmsg("constraint \"%s\" of domain \"%s\" is not a check constraint",
 						constrName, TypeNameToString(typename))));
 
-	val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin);
-	conbin = TextDatumGetCString(val);
+	if (!con->convalidated)
+	{
+		val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin);
+		conbin = TextDatumGetCString(val);
 
-	/*
-	 * Locking related relations with ShareUpdateExclusiveLock is ok because
-	 * not-yet-valid constraints are still enforced against concurrent inserts
-	 * or updates.
-	 */
-	validateDomainCheckConstraint(domainoid, conbin, ShareUpdateExclusiveLock);
+		/*
+		 * Locking related relations with ShareUpdateExclusiveLock is ok because
+		 * not-yet-valid constraints are still enforced against concurrent inserts
+		 * or updates.
+		 */
+		validateDomainCheckConstraint(domainoid, conbin, ShareUpdateExclusiveLock);
 
-	/*
-	 * Now update the catalog, while we have the door open.
-	 */
-	copyTuple = heap_copytuple(tuple);
-	copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
-	copy_con->convalidated = true;
-	CatalogTupleUpdate(conrel, &copyTuple->t_self, copyTuple);
+		/*
+		 * Now update the catalog, while we have the door open.
+		 */
+		copyTuple = heap_copytuple(tuple);
+		copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
+		copy_con->convalidated = true;
+		CatalogTupleUpdate(conrel, &copyTuple->t_self, copyTuple);
 
-	InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
+		InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
 
-	ObjectAddressSet(address, TypeRelationId, domainoid);
+		ObjectAddressSet(address, TypeRelationId, domainoid);
 
-	heap_freetuple(copyTuple);
+		heap_freetuple(copyTuple);
+	}
 
 	systable_endscan(scan);
 
-- 
2.34.1

Reply via email to