Hi. Context: https://git.postgresql.org/cgit/postgresql.git/commit/?id=3c5926301aea476025f118159688a6a88b2738bc Also see build_coercion_expression, COERCION_PATH_ARRAYCOERCE handling.
It's doable to skip a table rewrite when changing a column's data type from one array type to another. We just need some logic to handle ArrayCoerceExpr within ATColumnChangeRequiresRewrite. ArrayCoerceExpr have two node expression, ``arg`` and ``elemexpr``, therefore ATColumnChangeRequiresRewrite, the old single ``FOR(;;)`` loop is not enough, we need recursive walking through ArrayCoerceExpr->arg and ArrayCoerceExpr->elemexpr. Please see the attached POC. DEMO: create table rewriteme (id serial primary key, foo float, bar timestamptz); begin; alter table rewriteme add column ttz_arr timestamptz[]; set timezone to 'UTC'; alter table rewriteme alter column bar type timestamp; ---- no table rewrite with PATCH, require table rewrite in HEAD alter table rewriteme alter column ttz_arr type timestamp[]; rollback; CREATE DOMAIN domain1 as INT; CREATE TABLE test_set_col_type(a INT[]); ---- no table rewrite with PATCH, require table rewrite in HEAD ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain1[]; ---- no table rewrite with PATCH, require table rewrite in HEAD ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE INT[]; -- jian https://www.enterprisedb.com/
From 077b3cb290f7161f66621aaa756f2edc88d7a98a Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Wed, 18 Mar 2026 19:03:01 +0800 Subject: [PATCH v1 1/1] Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce Context: https://git.postgresql.org/cgit/postgresql.git/commit/?id=3c5926301aea476025f118159688a6a88b2738bc Discussion: https://postgr.es/m/ Commitfest: https://commitfest.postgresql.org/patch --- src/backend/commands/tablecmds.c | 126 +++++++++++++++----- src/test/regress/expected/event_trigger.out | 5 + src/test/regress/expected/fast_default.out | 13 ++ src/test/regress/sql/event_trigger.sql | 4 + src/test/regress/sql/fast_default.sql | 13 ++ 5 files changed, 132 insertions(+), 29 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 67e42e5df29..3d0f5b5ee39 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -659,6 +659,8 @@ static void ATPrepAlterColumnType(List **wqueue, AlterTableCmd *cmd, LOCKMODE lockmode, AlterTableUtilityContext *context); static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno); +static void ATColumnChangeRequiresRewriteWalker(Node *expr, AttrNumber varattno, + bool *need_rewrite); static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, @@ -14911,8 +14913,9 @@ ATPrepAlterColumnType(List **wqueue, * * - the old type is binary coercible to the new type * - the new type is an unconstrained domain over the old type + * * - the new type is an unconstrained domain array over the old type * - {NEW,OLD} or {OLD,NEW} is {timestamptz,timestamp} and the timezone is UTC - * + * - {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. @@ -14920,42 +14923,107 @@ ATPrepAlterColumnType(List **wqueue, static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno) { + bool need_rewrite = false; + Assert(expr != NULL); - for (;;) + ATColumnChangeRequiresRewriteWalker(expr, + varattno, + &need_rewrite); + + return need_rewrite; +} + +static void +ATColumnChangeRequiresRewriteWalker(Node *expr, AttrNumber varattno, + bool *need_rewrite) +{ + if (*need_rewrite) + return; + + if (IsA(expr, Var)) { - /* only one varno, so no need to check that */ - if (IsA(expr, Var) && ((Var *) expr)->varattno == varattno) - return false; - else if (IsA(expr, RelabelType)) - expr = (Node *) ((RelabelType *) expr)->arg; - else if (IsA(expr, CoerceToDomain)) + Var *var = (Var *) expr; + + if (var->varattno != varattno) { - CoerceToDomain *d = (CoerceToDomain *) expr; + *need_rewrite = true; + return; + } + } + else if (IsA(expr, RelabelType)) + { + RelabelType *r = (RelabelType *) expr; + + ATColumnChangeRequiresRewriteWalker((Node *) r->arg, + varattno, + need_rewrite); + + return; + } + else if (IsA(expr, ArrayCoerceExpr)) + { + ArrayCoerceExpr *acoerce = (ArrayCoerceExpr *) expr; + + ATColumnChangeRequiresRewriteWalker((Node *) acoerce->arg, + varattno, + need_rewrite); - if (DomainHasConstraints(d->resulttype, NULL)) - return true; - expr = (Node *) d->arg; + ATColumnChangeRequiresRewriteWalker((Node *) acoerce->elemexpr, + varattno, + need_rewrite); + + return; + } + else if (IsA(expr, CoerceToDomain)) + { + CoerceToDomain *d = (CoerceToDomain *) expr; + + if (DomainHasConstraints(d->resulttype, NULL)) + { + *need_rewrite = true; + return; } - else if (IsA(expr, FuncExpr)) + + ATColumnChangeRequiresRewriteWalker((Node *) d->arg, + varattno, + need_rewrite); + + return; + } + else if (IsA(expr, FuncExpr)) + { + FuncExpr *f = (FuncExpr *) expr; + + switch (f->funcid) { - FuncExpr *f = (FuncExpr *) expr; - - switch (f->funcid) - { - case F_TIMESTAMPTZ_TIMESTAMP: - case F_TIMESTAMP_TIMESTAMPTZ: - if (TimestampTimestampTzRequiresRewrite()) - return true; - else - expr = linitial(f->args); - break; - default: - return true; - } + case F_TIMESTAMPTZ_TIMESTAMP: + case F_TIMESTAMP_TIMESTAMPTZ: + if (TimestampTimestampTzRequiresRewrite()) + { + *need_rewrite = true; + return; + } + else + { + expr = linitial(f->args); + ATColumnChangeRequiresRewriteWalker(expr, + varattno, + need_rewrite); + return; + } + break; + default: + { + *need_rewrite = true; + return; + } } - else - return true; + } + else if (!IsA(expr, CaseTestExpr)) + { + *need_rewrite = true; + return; } } diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index f897b079e67..92f6238333c 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -609,13 +609,18 @@ DROP MATERIALIZED VIEW heapmv; -- shouldn't trigger a table_rewrite event alter table rewriteme alter column foo type numeric(12,4); begin; +alter table rewriteme add column zoo timestamptz[]; set timezone to 'UTC'; alter table rewriteme alter column bar type timestamp; +alter table rewriteme alter column zoo type timestamp[]; set timezone to '0'; alter table rewriteme alter column bar type timestamptz; +alter table rewriteme alter column zoo type timestamptz[]; set timezone to 'Europe/London'; alter table rewriteme alter column bar type timestamp; -- does rewrite NOTICE: Table 'rewriteme' is being rewritten (reason = 4) +alter table rewriteme alter column zoo type timestamp[]; -- does rewrite +NOTICE: Table 'rewriteme' is being rewritten (reason = 4) rollback; -- typed tables are rewritten when their type changes. Don't emit table -- name, because firing order is not stable. diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index ffbc47089b1..7321e975c55 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -384,6 +384,19 @@ DROP DOMAIN domain7; DROP DOMAIN domain8; DROP DOMAIN domain9; DROP FUNCTION foo(INT); +CREATE DOMAIN domain1 as INT; +CREATE DOMAIN domain2 as INT[]; +CREATE DOMAIN domain3 AS domain1[]; +CREATE TABLE test_set_col_type(a INT[], b INT[]); +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain1[]; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE INT[]; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain2 USING a; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain3; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE INT[]; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain3 using b; -- table rewrite +NOTICE: rewriting table test_set_col_type for reason 4 +DROP TABLE test_set_col_type; +DROP DOMAIN domain1, domain2, 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/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index 32e9bb58c5e..3d9b1f4f623 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -445,12 +445,16 @@ DROP MATERIALIZED VIEW heapmv; -- shouldn't trigger a table_rewrite event alter table rewriteme alter column foo type numeric(12,4); begin; +alter table rewriteme add column zoo timestamptz[]; set timezone to 'UTC'; alter table rewriteme alter column bar type timestamp; +alter table rewriteme alter column zoo type timestamp[]; set timezone to '0'; alter table rewriteme alter column bar type timestamptz; +alter table rewriteme alter column zoo type timestamptz[]; set timezone to 'Europe/London'; alter table rewriteme alter column bar type timestamp; -- does rewrite +alter table rewriteme alter column zoo type timestamp[]; -- does rewrite rollback; -- typed tables are rewritten when their type changes. Don't emit table diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 8ff29cf2697..7f1fb23c4e5 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -345,6 +345,19 @@ DROP DOMAIN domain8; DROP DOMAIN domain9; DROP FUNCTION foo(INT); +CREATE DOMAIN domain1 as INT; +CREATE DOMAIN domain2 as INT[]; +CREATE DOMAIN domain3 AS domain1[]; +CREATE TABLE test_set_col_type(a INT[], b INT[]); +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain1[]; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE INT[]; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain2 USING a; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain3; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE INT[]; -- no table rewrite +ALTER TABLE test_set_col_type ALTER COLUMN a SET DATA TYPE domain3 using b; -- table rewrite +DROP TABLE test_set_col_type; +DROP DOMAIN domain1, domain2, domain3; + -- Fall back to full rewrite for volatile expressions CREATE TABLE T(pk INT NOT NULL PRIMARY KEY); -- 2.34.1
