On Wed, Oct 1, 2025 at 11:04 AM jian he <[email protected]> wrote:
>
> we can simply change from
>
> if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION)
> ATPostAlterTypeCleanup(wqueue, tab, lockmode);
>
> to
> if (pass == AT_PASS_SET_EXPRESSION)
> ATPostAlterTypeCleanup(wqueue, tab, lockmode);
> else if (pass == AT_PASS_ALTER_TYPE &&
> tab->subcmds[AT_PASS_SET_EXPRESSION] == NIL)
> ATPostAlterTypeCleanup(wqueue, tab, lockmode);
That will not work if AT_PASS_ALTER_TYPE triggers the creation of a new
AT_PASS_SET_EXPRESSION entry.
For example, consider:
CREATE TABLE gtest25 (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
If we alter the type of column "a", the column b’s generation
expression would need
to be rebuilt (which is currently not supported). In that case, the current
logic would not be able to handle it.
I think the correct fix is within ATPostAlterTypeCleanup.
at the end of ATPostAlterTypeCleanup, we already delete these changed
objects via
``performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);``
we can just list_free these (the below) objects too:
tab->changedConstraintOids
tab->changedConstraintDefs
tab->changedIndexOids
tab->changedIndexDefs
tab->changedStatisticsOids
tab->changedStatisticsDefs
since this information would become stale if those objects have
already been dropped.
From 4ab21b82da7ff32b8b37a51a1f8ee5079ee2b601 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Mon, 13 Oct 2025 16:05:12 +0800
Subject: [PATCH v5 1/1] avoid call ATPostAlterTypeCleanup twice
should we use list_free_deep for tab->changedConstraintDefs,
tab->changedIndexDefs, tab->changedStatisticsDefs?
discussion: https://postgr.es/m/cacjufxhzsgn3zm5g-x7ymtfgzndnrwr07s+gyfius+tz45m...@mail.gmail.com
---
src/backend/commands/tablecmds.c | 18 ++++++++++++
.../regress/expected/generated_stored.out | 29 +++++++++++++++++++
.../regress/expected/generated_virtual.out | 26 +++++++++++++++++
src/test/regress/sql/generated_stored.sql | 8 +++++
src/test/regress/sql/generated_virtual.sql | 7 +++++
5 files changed, 88 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5fd8b51312c..5c86fba48d9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15577,6 +15577,24 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
free_object_addresses(objects);
+ /*
+ * We have already deleted the dependent objects; now remove these objects
+ * themselves to avoid deleting them twice.
+ */
+ list_free(tab->changedConstraintOids);
+ list_free(tab->changedConstraintDefs);
+ list_free(tab->changedIndexOids);
+ list_free(tab->changedIndexDefs);
+ list_free(tab->changedStatisticsOids);
+ list_free(tab->changedStatisticsDefs);
+
+ tab->changedConstraintOids = NIL;
+ tab->changedConstraintDefs = NIL;
+ tab->changedIndexOids = NIL;
+ tab->changedIndexDefs = NIL;
+ tab->changedStatisticsOids = NIL;
+ tab->changedStatisticsDefs = NIL;
+
/*
* The objects will get recreated during subsequent passes over the work
* queue.
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index adac2cedfb2..d34a5839680 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -1120,6 +1120,35 @@ SELECT * FROM gtest25 ORDER BY a;
Indexes:
"gtest25_pkey" PRIMARY KEY, btree (a)
+ALTER TABLE gtest25 ADD COLUMN z INT DEFAULT 11;
+ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID;
+ALTER TABLE gtest25 ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID;
+ALTER TABLE gtest25 ALTER COLUMN z SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 1);
+ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 2);
+SELECT * FROM gtest25 ORDER BY a;
+ a | b | c | x | d | y | z
+---+----+----+-----+-----+-----+----
+ 3 | 13 | 42 | 168 | 101 | 404 | 11
+ 4 | 13 | 42 | 168 | 101 | 404 | 11
+(2 rows)
+
+\d gtest25
+ Table "generated_stored_tests.gtest25"
+ Column | Type | Collation | Nullable | Default
+--------+------------------+-----------+----------+------------------------------------------------------
+ a | integer | | not null |
+ b | numeric | | | generated always as (z + 2::numeric) stored
+ c | integer | | | 42
+ x | integer | | | generated always as (c * 4) stored
+ d | double precision | | | 101
+ y | double precision | | | generated always as (d * 4::double precision) stored
+ z | numeric | | | 11
+Indexes:
+ "gtest25_pkey" PRIMARY KEY, btree (a)
+Check constraints:
+ "cc" CHECK (b > 9::numeric) NOT VALID
+ "check_z" CHECK (z > 9::numeric) NOT VALID
+
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
a int,
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index c861bd36c5a..faac85bd769 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1082,6 +1082,32 @@ SELECT * FROM gtest25 ORDER BY a;
Indexes:
"gtest25_pkey" PRIMARY KEY, btree (a)
+ALTER TABLE gtest25 ADD COLUMN z INT DEFAULT 11;
+-- ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID; --ok
+-- ALTER TABLE gtest25 ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID; --ok
+ALTER TABLE gtest25 ALTER COLUMN z SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 1);
+ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 2);
+SELECT * FROM gtest25 ORDER BY a;
+ a | b | c | x | d | y | z
+---+----+----+-----+-----+-----+----
+ 3 | 13 | 42 | 168 | 101 | 404 | 11
+ 4 | 13 | 42 | 168 | 101 | 404 | 11
+(2 rows)
+
+\d gtest25
+ Table "generated_virtual_tests.gtest25"
+ Column | Type | Collation | Nullable | Default
+--------+------------------+-----------+----------+-----------------------------------------------
+ a | integer | | not null |
+ b | numeric | | | generated always as (z + 2::numeric)
+ c | integer | | | 42
+ x | integer | | | generated always as (c * 4)
+ d | double precision | | | 101
+ y | double precision | | | generated always as (d * 4::double precision)
+ z | numeric | | | 11
+Indexes:
+ "gtest25_pkey" PRIMARY KEY, btree (a)
+
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
a int,
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index f56fde8d4e5..9fe2d2ca04c 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -516,6 +516,14 @@ ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8,
ADD COLUMN y float8 GENERATED ALWAYS AS (d * 4) STORED;
SELECT * FROM gtest25 ORDER BY a;
\d gtest25
+ALTER TABLE gtest25 ADD COLUMN z INT DEFAULT 11;
+ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID;
+ALTER TABLE gtest25 ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID;
+ALTER TABLE gtest25 ALTER COLUMN z SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 1);
+ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 2);
+SELECT * FROM gtest25 ORDER BY a;
+\d gtest25
+
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index adfe88d74ae..0e19d0640d9 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -559,6 +559,13 @@ ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8,
ADD COLUMN y float8 GENERATED ALWAYS AS (d * 4) VIRTUAL;
SELECT * FROM gtest25 ORDER BY a;
\d gtest25
+ALTER TABLE gtest25 ADD COLUMN z INT DEFAULT 11;
+-- ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID; --ok
+-- ALTER TABLE gtest25 ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID; --ok
+ALTER TABLE gtest25 ALTER COLUMN z SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 1);
+ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (z + 2);
+SELECT * FROM gtest25 ORDER BY a;
+\d gtest25
-- ALTER TABLE ... ALTER COLUMN
CREATE TABLE gtest27 (
--
2.34.1
