hi.

the attached patch is to implement the $subject feature.
i was mainly intrigued by the discussion in
https://www.postgresql.org/message-id/20190226061450.ga1665...@rfd.leadboat.com

the main gotcha is struct NewColumnValue.
we do ``palloc0(sizeof(NewColumnValue));`` in ATExecAddColumn,
ATExecSetExpression, ATPrepAlterColumnType.

ATExecAddColumn:  Adding a new column with domain with constraints will cause
                  table rewrite.
ATExecSetExpression: for stored generated column will cause table rewrite, we do
                     not support domain over virtual generated columns now.
ATPrepAlterColumnType: we only do table rewriting occasionally.
see ATColumnChangeRequiresRewrite.

If table rewrite is required, then there is nothing we can do. so
we only need to focus on ATPrepAlterColumnType.
we can add a new boolean field, coerce_to_domain, to NewColumnValue. this field
is set to true only when changing an existing column's type to a constrained
domain. In such cases, a table scan is enough—no table rewrite is needed.
coerce_to_domain will set to false, if table rewrite is required.
From cad4a214c46dfd95fa2ee5c34e5804bd565bd4fb Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Thu, 10 Jul 2025 01:52:25 +0800
Subject: [PATCH v1 1/1] no table rewrite when set column type to constrained
 domain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

per https://www.postgresql.org/docs/devel/sql-altertable.html
says changing the type of an existing column to a constrained domain
will trigger a table rewrite. However, after reviewing the relevant context [1]
and experimenting with it, I found that it's doable to just a table scan.

the main gotcha is struct NewColumnValue.
we do ``palloc0(sizeof(NewColumnValue));`` in ATExecAddColumn,
ATExecSetExpression, ATPrepAlterColumnType.
ATExecAddColumn:  Adding a new column with domain with constraints will cause
                  table rewrite.
ATExecSetExpression: for stored generated column will cause table rewrite, we do
                     not support domain over virtual generated columns now.
ATPrepAlterColumnType: we only do table rewriting occasionally.see ATColumnChangeRequiresRewrite.

If table rewrite is required, then there is nothing we can do.
we can add a new boolean field, coerce_to_domain, to NewColumnValue. this field
is set to true only when changing an existing column's type to a constrained
domain. In such cases, a table scan is enough—no table rewrite is needed.

[1]: https://www.postgresql.org/message-id/20190226061450.ga1665...@rfd.leadboat.com
discussion: https://postgr.es/m/XXX
commitfest: https://commitfest.postgresql.org/patch/XXX
---
 doc/src/sgml/ref/alter_table.sgml          |  6 +--
 src/backend/commands/tablecmds.c           | 60 +++++++++++++++++++---
 src/test/regress/expected/fast_default.out | 31 +++++++++++
 src/test/regress/sql/fast_default.sql      | 27 ++++++++++
 4 files changed, 114 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1e4f26c13f6..20a99758d12 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1447,9 +1447,9 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     and its indexes to be rewritten.
     As an exception, when changing the type of an existing column,
     if the <literal>USING</literal> clause does not change the column
-    contents and the old type is either binary coercible to the new type
-    or an unconstrained domain over the new type, a table rewrite is not
-    needed.  However, indexes will still be rebuilt unless the system
+    contents and the old type is either binary coercible to the new type,
+    a table rewrite is not needed.
+    However, indexes will still be rebuilt unless the system
     can verify that the new index would be logically equivalent to the
     existing one.  For example, if the collation for a column has been
     changed, an index rebuild is required because the new sort
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b8837f26cb4..5d642e76257 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -237,6 +237,8 @@ typedef struct NewColumnValue
 	Expr	   *expr;			/* expression to compute */
 	ExprState  *exprstate;		/* execution state */
 	bool		is_generated;	/* is it a GENERATED expression? */
+	/* is it coerce to domain, this is only for changing column data type */
+	bool		coerce_to_domain;
 } NewColumnValue;
 
 /*
@@ -6009,7 +6011,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
 			 * rebuild data.
 			 */
 			if (tab->constraints != NIL || tab->verify_new_notnull ||
-				tab->partition_constraint != NULL)
+				tab->partition_constraint != NULL || tab->newvals)
 				ATRewriteTable(tab, InvalidOid);
 
 			/*
@@ -6119,7 +6121,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 	Relation	newrel;
 	TupleDesc	oldTupDesc;
 	TupleDesc	newTupDesc;
+	TupleDesc 	old_tmp;
 	bool		needscan = false;
+	bool		coerce_to_domain = false;
 	List	   *notnull_attrs;
 	List	   *notnull_virtual_attrs;
 	int			i;
@@ -6137,7 +6141,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 	oldrel = table_open(tab->relid, NoLock);
 	oldTupDesc = tab->oldDesc;
 	newTupDesc = RelationGetDescr(oldrel);	/* includes all mods */
-
+	old_tmp = CreateTupleDescCopy(oldTupDesc);
 	if (OidIsValid(OIDNewHeap))
 	{
 		Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,
@@ -6204,6 +6208,11 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 
 		/* expr already planned */
 		ex->exprstate = ExecInitExpr((Expr *) ex->expr, NULL);
+		if (ex->coerce_to_domain && !tab->rewrite && !coerce_to_domain)
+		{
+			needscan = true;
+			coerce_to_domain = true;
+		}
 	}
 
 	notnull_attrs = notnull_virtual_attrs = NIL;
@@ -6432,6 +6441,42 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
 				 * new constraints etc.
 				 */
 				insertslot = oldslot;
+
+				/*
+				 * The tupdesc (newTupDesc) in oldslot already includes the
+				 * updated attribute changes.  If we use it in ExecEvalExpr,
+				 * CheckVarSlotCompatibility will fail.  Therefore, we need to
+				 * temporarily set oldslot's tts_tupleDescriptor to oldTupDesc.
+				 *
+				 * coerce_to_domain means a AT_AlterColumnType  where a column's
+				 * type is being changed to a domain. Here we doesn't require
+				 * table rewrite, but it needs to verify that existing column
+				 * values can be coerced to the domain.
+				*/
+				if (coerce_to_domain)
+				{
+					Datum		values pg_attribute_unused();
+					bool		isnull pg_attribute_unused();
+					insertslot->tts_tupleDescriptor = old_tmp;
+					econtext->ecxt_scantuple = insertslot;
+
+					foreach(l, tab->newvals)
+					{
+						NewColumnValue *ex = lfirst(l);
+
+						if (!ex->coerce_to_domain)
+							continue;
+
+						/*
+						 * we can not use ExecEvalExprNoReturn here, because we
+						 * use ExecInitExpr compile NewColumnValue->expr.
+						 */
+						values = ExecEvalExpr(ex->exprstate, econtext, &isnull);
+						values = (Datum) 0;
+						isnull = true;
+					}
+					insertslot->tts_tupleDescriptor = newTupDesc;
+				}
 			}
 
 			/* Now check any constraints on the possibly-changed tuple */
@@ -7509,6 +7554,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			newval->attnum = attribute->attnum;
 			newval->expr = defval;
 			newval->is_generated = (colDef->generated != '\0');
+			newval->coerce_to_domain = false;
 
 			tab->newvals = lappend(tab->newvals, newval);
 
@@ -8698,6 +8744,7 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName,
 		newval->attnum = attnum;
 		newval->expr = expression_planner(defval);
 		newval->is_generated = true;
+		newval->coerce_to_domain = false;
 
 		tab->newvals = lappend(tab->newvals, newval);
 		tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
@@ -14499,10 +14546,13 @@ ATPrepAlterColumnType(List **wqueue,
 		newval->attnum = attnum;
 		newval->expr = (Expr *) transform;
 		newval->is_generated = false;
+		newval->coerce_to_domain = DomainHasConstraints(targettype);
 
 		tab->newvals = lappend(tab->newvals, newval);
 		if (ATColumnChangeRequiresRewrite(transform, attnum))
 			tab->rewrite |= AT_REWRITE_COLUMN_REWRITE;
+		if (tab->rewrite)
+			newval->coerce_to_domain = false;
 	}
 	else if (transform)
 		ereport(ERROR,
@@ -14633,12 +14683,10 @@ ATPrepAlterColumnType(List **wqueue,
  * rewrite in these cases:
  *
  * - the old type is binary coercible to the new type
- * - the new type is an unconstrained domain over the old type
  * - {NEW,OLD} or {OLD,NEW} is {timestamptz,timestamp} and the timezone is UTC
  *
  * In the case of a constrained domain, we could get by with scanning the
- * table and checking the constraint rather than actually rewriting it, but we
- * don't currently try to do that.
+ * table and checking the constraint rather than actually rewriting it.
  */
 static bool
 ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno)
@@ -14656,8 +14704,6 @@ ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno)
 		{
 			CoerceToDomain *d = (CoerceToDomain *) expr;
 
-			if (DomainHasConstraints(d->resulttype))
-				return true;
 			expr = (Node *) d->arg;
 		}
 		else if (IsA(expr, FuncExpr))
diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out
index ccbcdf8403f..9c844ed4373 100644
--- a/src/test/regress/expected/fast_default.out
+++ b/src/test/regress/expected/fast_default.out
@@ -323,6 +323,37 @@ DROP DOMAIN domain2;
 DROP DOMAIN domain3;
 DROP DOMAIN domain4;
 DROP FUNCTION foo(INT);
+-- Test domains with default value for table rewrite.
+CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain2 AS INT CHECK(VALUE > random(min=>10, max=>10)) NOT NULL;
+CREATE TABLE t22(a INT, CONSTRAINT cc CHECK(a > 1), b domain1, CONSTRAINT cc1 CHECK(b > 1));
+INSERT INTO t22 VALUES(NULL, 2), (2, 3);
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1;
+ERROR:  domain domain1 does not allow null values
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain2;
+ERROR:  value for domain domain2 violates check constraint "domain2_check"
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING a::INT::domain2::domain1;
+ERROR:  domain domain2 does not allow null values
+-- table rewrite should happen
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (a+1)::domain1;
+NOTICE:  rewriting table t22 for reason 4
+ERROR:  domain domain1 does not allow null values
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (COALESCE(a, 1) +1)::domain1;
+NOTICE:  rewriting table t22 for reason 4
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain2;
+ERROR:  value for domain domain2 violates check constraint "domain2_check"
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain2;
+ERROR:  value for domain domain2 violates check constraint "domain2_check"
+-- table rewrite should happen
+ALTER TABLE t22
+  ALTER COLUMN a SET DATA TYPE domain2 USING (a +9)::domain2,
+  ALTER COLUMN b SET DATA TYPE domain2 USING (b +9)::domain2;
+NOTICE:  rewriting table t22 for reason 4
+DROP TABLE t22;
+DROP DOMAIN domain1;
+DROP DOMAIN domain2;
 -- Fall back to full rewrite for volatile expressions
 CREATE TABLE T(pk INT NOT NULL PRIMARY KEY);
 INSERT INTO T VALUES (1);
diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql
index 068dd0bc8aa..21440bff294 100644
--- a/src/test/regress/sql/fast_default.sql
+++ b/src/test/regress/sql/fast_default.sql
@@ -294,6 +294,33 @@ DROP DOMAIN domain3;
 DROP DOMAIN domain4;
 DROP FUNCTION foo(INT);
 
+-- Test domains with default value for table rewrite.
+CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain2 AS INT CHECK(VALUE > random(min=>10, max=>10)) NOT NULL;
+CREATE TABLE t22(a INT, CONSTRAINT cc CHECK(a > 1), b domain1, CONSTRAINT cc1 CHECK(b > 1));
+INSERT INTO t22 VALUES(NULL, 2), (2, 3);
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1;
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain2;
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING a::INT::domain2::domain1;
+
+-- table rewrite should happen
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (a+1)::domain1;
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (COALESCE(a, 1) +1)::domain1;
+
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain2;
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain2;
+
+-- table rewrite should happen
+ALTER TABLE t22
+  ALTER COLUMN a SET DATA TYPE domain2 USING (a +9)::domain2,
+  ALTER COLUMN b SET DATA TYPE domain2 USING (b +9)::domain2;
+
+DROP TABLE t22;
+DROP DOMAIN domain1;
+DROP DOMAIN domain2;
+
 -- Fall back to full rewrite for volatile expressions
 CREATE TABLE T(pk INT NOT NULL PRIMARY KEY);
 
-- 
2.34.1

Reply via email to