On Tue, Aug 26, 2025 at 11:26 AM jian he <[email protected]> wrote:
>
> typedef struct NewColumnValue
> {
> AttrNumber attnum; /* which column */
> Expr *expr; /* expression to compute */
> ExprState *exprstate; /* execution state */
> bool is_generated; /* is it a GENERATED expression? */
> bool scan_only; /* table scan only */
> } NewColumnValue;
I changed scan_only to need_compute.
+ *
+ * If need_compute is true, we will evaluate the new column value in Phase 3.
+ * Currently, this is only used in ALTER COLUMN SET DATA TYPE
command, where the
+ * column’s data type is being changed to a constrained domain, and all the
+ * domain's constraints are non-volatile. In case table rewrite, we also set it
+ * to true.
*/
typedef struct NewColumnValue
{
@@ -238,6 +244,7 @@ typedef struct NewColumnValue
Expr *expr; /* expression to compute */
ExprState *exprstate; /* execution state */
bool is_generated; /* is it a GENERATED expression? */
+ bool need_compute; /* compute this new expression in Phase 3 */
} NewColumnValue;
I use domain over domain for regress tests.
I also constrained the no–table-rewrite behavior to cases where the coercion is
to a domain type and all constraints of the new domain are non-volatile.
Demo:
CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
CREATE DOMAIN domain11 AS domain1 CHECK(VALUE > 1) NOT NULL;
CREATE DOMAIN domain21 AS domain1 CHECK(VALUE > random(min=>10,
max=>10)) NOT NULL;
CREATE DOMAIN domain3 AS INT8;
CREATE TABLE t22(a INT, b INT);
INSERT INTO t22 VALUES(-2, -1);
-- no table rewrite, but fail at domain constraint check
ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain11 USING a::domain11;
-- no table rewrite, but fail at domain constraint check
ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain11 USING b::domain11;
-- table rewrite
ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain21;
ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain3;
ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (a+0)::domain1;
--
jian
https://www.enterprisedb.com
From ced739be763b707f8b90bc253c10c3b10905214b Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Mon, 15 Dec 2025 14:51:58 +0800
Subject: [PATCH v3 2/2] skip table rewrite when set column type to constrained
domain
If a table rewrite is required, there's nothing we can do about it. We can add
a new boolean field need_compute to NewColumnValue.
This field is currently set to true only when changing an existing column's type
to a constrained domain. In such cases, a table scan alone is sufficient.
This only works if the new domain type all constraint are non-volatile. and new
domain base type is binary coercible with the old column type.
[1]: https://www.postgresql.org/message-id/[email protected]
discussion: https://postgr.es/m/cacjufxfx0dupbf5+dbnf3mxcfntz1y7jpt11+tcd_fcyadh...@mail.gmail.com
commitfest: https://commitfest.postgresql.org/patch/5907
---
doc/src/sgml/ref/alter_table.sgml | 5 +-
src/backend/commands/tablecmds.c | 87 ++++++++++++++++++++--
src/test/regress/expected/fast_default.out | 30 ++++++++
src/test/regress/sql/fast_default.sql | 30 ++++++++
4 files changed, 142 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 9abd8037f28..c0ece49ce68 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1664,8 +1664,9 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
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
+ or an unconstrained domain over the new type, or domain over new type has no
+ volatile constraint, 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 6b1a00ed477..5549fb0b192 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -231,6 +231,12 @@ typedef struct NewConstraint
* are just copied from the old table during ATRewriteTable. Note that the
* expr is an expression over *old* table values, except when is_generated
* is true; then it is an expression over columns of the *new* tuple.
+ *
+ * If need_compute is true, we will evaluate the new column value in Phase 3.
+ * Currently, this is only used in ALTER COLUMN SET DATA TYPE command, where the
+ * column’s data type is being changed to a constrained domain, and all the
+ * domain's constraints are non-volatile. In case table rewrite, we also set it
+ * to true.
*/
typedef struct NewColumnValue
{
@@ -238,6 +244,7 @@ typedef struct NewColumnValue
Expr *expr; /* expression to compute */
ExprState *exprstate; /* execution state */
bool is_generated; /* is it a GENERATED expression? */
+ bool need_compute; /* compute this new expression in Phase 3 */
} NewColumnValue;
/*
@@ -6043,7 +6050,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);
/*
@@ -6153,6 +6160,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
Relation newrel;
TupleDesc oldTupDesc;
TupleDesc newTupDesc;
+ TupleDesc old_tmp;
bool needscan = false;
List *notnull_attrs;
List *notnull_virtual_attrs;
@@ -6171,7 +6179,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,
@@ -6238,6 +6246,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
/* expr already planned */
ex->exprstate = ExecInitExpr(ex->expr, NULL);
+
+ if (ex->need_compute)
+ needscan = true;
}
notnull_attrs = notnull_virtual_attrs = NIL;
@@ -6466,6 +6477,44 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
* new constraints etc.
*/
insertslot = oldslot;
+
+ /*
+ * newTupDesc for oldslot already have the updated attribute
+ * changes, but we cann't use it in ExecEvalExpr, otherwise
+ * CheckVarSlotCompatibility will fail. Therefore, we
+ * temporarily set oldslot's tts_tupleDescriptor to oldTupDesc.
+ * Essentially, what we're doing here is evaluating the
+ * CoerceToDomain node against the existing table slot.
+ */
+ if (tab->newvals != NIL)
+ {
+ 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->need_compute)
+ continue;
+
+ /*
+ * we can not use ExecEvalExprNoReturn here, because we
+ * use ExecInitExpr compile NewColumnValue->expr. Here,
+ * we only check whether the oldslot value satisfies the
+ * domain constraint. So it is ok to override the value
+ * evaluated by ExecEvalExpr.
+ */
+ values = ExecEvalExpr(ex->exprstate, econtext, &isnull);
+ values = (Datum) 0;
+ isnull = true;
+ }
+
+ insertslot->tts_tupleDescriptor = newTupDesc;
+ }
}
/* Now check any constraints on the possibly-changed tuple */
@@ -14562,10 +14611,36 @@ ATPrepAlterColumnType(List **wqueue,
newval->attnum = attnum;
newval->expr = (Expr *) transform;
newval->is_generated = false;
+ newval->need_compute = false;
- tab->newvals = lappend(tab->newvals, newval);
if (ATColumnChangeRequiresRewrite(transform, attnum))
+ {
tab->rewrite |= AT_REWRITE_COLUMN_REWRITE;
+ newval->need_compute = true;
+ }
+
+ /*
+ * If the new column type is a domain and the domain's base type is
+ * binary coercible with the old column type. We already set
+ * need_compute to true in case of table rewrite.
+ */
+ if (!tab->rewrite &&IsA(transform, CoerceToDomain))
+ {
+ bool has_volatile = false;
+
+ CoerceToDomain *d = (CoerceToDomain *) transform;
+
+ (void) DomainHaveVolatileConstraints(targettype, &has_volatile);
+
+ if (has_volatile)
+ {
+ tab->rewrite |= AT_REWRITE_COLUMN_REWRITE;
+ newval->need_compute = true;
+ }
+ else if (IsA(d->arg, Var))
+ newval->need_compute = true;
+ }
+ tab->newvals = lappend(tab->newvals, newval);
}
else if (transform)
ereport(ERROR,
@@ -14696,12 +14771,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)
@@ -14719,8 +14792,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..eff8f6dc450 100644
--- a/src/test/regress/expected/fast_default.out
+++ b/src/test/regress/expected/fast_default.out
@@ -323,6 +323,36 @@ DROP DOMAIN domain2;
DROP DOMAIN domain3;
DROP DOMAIN domain4;
DROP FUNCTION foo(INT);
+-- Test change column data type to domain for table rewrite
+CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain11 AS domain1 CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain21 AS domain1 CHECK(VALUE > random(min=>10, max=>10)) NOT NULL;
+CREATE DOMAIN domain3 AS INT8;
+CREATE TABLE t22(a INT, b INT);
+INSERT INTO t22 VALUES(-2, -1);
+-- no table rewrite, but fail at domain constraint check
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain11 USING a::domain11;
+ERROR: value for domain domain11 violates check constraint "domain1_check"
+-- no table rewrite, but fail at domain constraint check
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain11 USING b::domain11;
+ERROR: value for domain domain11 violates check constraint "domain1_check"
+-- table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain21;
+NOTICE: rewriting table t22 for reason 4
+ERROR: value for domain domain21 violates check constraint "domain1_check"
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain3;
+NOTICE: rewriting table t22 for reason 4
+TRUNCATE t22;
+INSERT INTO t22 VALUES(2, -1);
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain11;
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE INT;
+-- table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (a+0)::domain1;
+NOTICE: rewriting table t22 for reason 4
+DROP TABLE t22;
+DROP DOMAIN domain21, domain11, domain1, domain3;
-- 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..52acbb0c5e4 100644
--- a/src/test/regress/sql/fast_default.sql
+++ b/src/test/regress/sql/fast_default.sql
@@ -294,6 +294,36 @@ DROP DOMAIN domain3;
DROP DOMAIN domain4;
DROP FUNCTION foo(INT);
+-- Test change column data type to domain for table rewrite
+CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain11 AS domain1 CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain21 AS domain1 CHECK(VALUE > random(min=>10, max=>10)) NOT NULL;
+CREATE DOMAIN domain3 AS INT8;
+CREATE TABLE t22(a INT, b INT);
+INSERT INTO t22 VALUES(-2, -1);
+
+-- no table rewrite, but fail at domain constraint check
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain11 USING a::domain11;
+
+-- no table rewrite, but fail at domain constraint check
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain11 USING b::domain11;
+
+-- table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain21;
+ALTER TABLE t22 ALTER COLUMN b SET DATA TYPE domain3;
+
+TRUNCATE t22;
+INSERT INTO t22 VALUES(2, -1);
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain11;
+-- no table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE INT;
+
+-- table rewrite
+ALTER TABLE t22 ALTER COLUMN a SET DATA TYPE domain1 USING (a+0)::domain1;
+DROP TABLE t22;
+DROP DOMAIN domain21, domain11, domain1, domain3;
+
-- Fall back to full rewrite for volatile expressions
CREATE TABLE T(pk INT NOT NULL PRIMARY KEY);
--
2.34.1
From 3781d439a33727aee57fe6345804ed04a9758cfc Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Mon, 19 May 2025 11:11:01 +0800
Subject: [PATCH v3 1/2] add function DomainHaveVolatileConstraints
the signature is:
extern bool DomainHaveVolatileConstraints(Oid type_id, bool *have_volatile);
Returns true if the Domain has any constraints. If you want check this domain
have any volatile check constraints, make sure have_volatile is not NULL.
discussion: https://postgr.es/m/cacjufxe_+izbr1i49k_ahigpppwltji6km8nosc7fwvkdem...@mail.gmail.com
---
src/backend/utils/cache/typcache.c | 40 ++++++++++++++++++++++++++++++
src/include/utils/typcache.h | 1 +
2 files changed, 41 insertions(+)
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 0c17d99d021..e449f59b3b6 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1499,6 +1499,46 @@ DomainHasConstraints(Oid type_id)
}
+/*
+ * Check whether a domain has any constraints, and determine if any of those
+ * constraints contain volatile expressions.
+ *
+ * To detect volatile expressions within domain check constraints, ensure that
+ * have_volatile is not NULL. If have_volatile is NULL, the behavior is
+ * equivalent to that of DomainHasConstraints.
+ */
+bool
+DomainHaveVolatileConstraints(Oid type_id, bool *have_volatile)
+{
+ TypeCacheEntry *typentry;
+
+ /*
+ * Note: a side effect is to cause the typcache's domain data to become
+ * valid. This is fine since we'll likely need it soon if there is any.
+ */
+ typentry = lookup_type_cache(type_id, TYPECACHE_DOMAIN_CONSTR_INFO);
+
+ if (typentry->domainData != NULL)
+ {
+ ListCell *lc;
+
+ foreach(lc, typentry->domainData->constraints)
+ {
+ DomainConstraintState *r = (DomainConstraintState *) lfirst(lc);
+
+ if (r->constrainttype == DOM_CONSTRAINT_CHECK &&
+ contain_volatile_functions((Node *) r->check_expr))
+ {
+ if (have_volatile)
+ *have_volatile = true;
+ break;
+ }
+ }
+ return true;
+ }
+ return false;
+}
+
/*
* array_element_has_equality and friends are helper routines to check
* whether we should believe that array_eq and related functions will work
diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h
index 1cb30f1818c..aa1c86e35c3 100644
--- a/src/include/utils/typcache.h
+++ b/src/include/utils/typcache.h
@@ -184,6 +184,7 @@ extern void InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref,
extern void UpdateDomainConstraintRef(DomainConstraintRef *ref);
extern bool DomainHasConstraints(Oid type_id);
+extern bool DomainHaveVolatileConstraints(Oid type_id, bool *have_volatile);
extern TupleDesc lookup_rowtype_tupdesc(Oid type_id, int32 typmod);
--
2.34.1
