public inbox for [email protected]
help / color / mirror / Atom feedFrom: jian he <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce
Date: Wed, 18 Mar 2026 19:06:14 +0800
Message-ID: <CACJufxE9rmyJv4a-gBxMykFPgzom1OYHp0XnNSYGfzEKQA=N_w@mail.gmail.com> (raw)
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/
Attachments:
[text/x-patch] v1-0001-Avoid-some-table-rewrites-for-ALTER-TABLE-.-SET-DATA-TYPE-array-c.patch (9.5K, 2-v1-0001-Avoid-some-table-rewrites-for-ALTER-TABLE-.-SET-DATA-TYPE-array-c.patch)
download | inline diff:
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
view thread (2+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce
In-Reply-To: <CACJufxE9rmyJv4a-gBxMykFPgzom1OYHp0XnNSYGfzEKQA=N_w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox