public inbox for [email protected]
help / color / mirror / Atom feedAvoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce
2+ messages / 2 participants
[nested] [flat]
* Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce
@ 2026-03-18 11:06 jian he <[email protected]>
2026-03-18 11:47 ` Re: Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce Matthias van de Meent <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: jian he @ 2026-03-18 11:06 UTC (permalink / raw)
To: pgsql-hackers
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
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce
2026-03-18 11:06 Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce jian he <[email protected]>
@ 2026-03-18 11:47 ` Matthias van de Meent <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Matthias van de Meent @ 2026-03-18 11:47 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers
On Wed, 18 Mar 2026 at 12:06, jian he <[email protected]> wrote:
>
> 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.
Are you sure about that?
IIRC, arrays store the element data type's OID on disk (^1), and if we
move from arrays of type X to arrays of type Y then that should also
change the array's on-disk OID: Generic array functions like array_out
rely on the array's type oid to select the lower-level functions to
call into when recursing; and would get the wrong type information if
we don't update this.
This could cause visible differences in output when you change the
time zone of your database after changing the column type from e.g.
timestamptz[] to timestamp[], or enum[] to oid[] if we ever implement
a binary coercible cast from enum to oid (or int, but oid seems more
appropriate).
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
(^1): Possibly DOMAIN-typed arrays only store the base type, in which
case this would work in the restricted case of coercing to
domain-typed arrays, but I don't think the general case of
binary-coercible types is allowable.
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-03-18 11:47 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-18 11:06 Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE array coerce jian he <[email protected]>
2026-03-18 11:47 ` Matthias van de Meent <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox