From d368f253b2bd0c1dfe61ee9c9d6ffa88940bbe9a Mon Sep 17 00:00:00 2001 From: jian he Date: Tue, 1 Jul 2025 11:17:11 +0800 Subject: [PATCH v3 1/1] fix-bug-18970 ALTER TYPE or SET EXPRESSION may necessitate rebuilding constraints, indexes, or statistics for related tables. As we may not acquire locks on these related relations before ATPostAlterTypeCleanup, we obtain appropriate locks here. When ALTER TYPE or SET EXPRESSION triggers a statistics rebuild for another table, we use ShareUpdateExclusiveLock to lock the related table, consistent with the locking approach in RemoveStatisticsById, AT_SetStatistics, and CreateStatistics. discussion: https://postgr.es/m/18970-a7d1cfe1f8d5d8d9@postgresql.org --- src/backend/commands/tablecmds.c | 12 ++++++++++++ src/test/regress/expected/alter_table.out | 8 ++++++++ src/test/regress/expected/generated_stored.out | 12 ++++++++++++ src/test/regress/sql/alter_table.sql | 8 ++++++++ src/test/regress/sql/generated_stored.sql | 13 +++++++++++++ 5 files changed, 53 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b8837f26cb4..f174a14d86b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -15488,6 +15488,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) Oid relid; relid = IndexGetRelation(oldId, false); + + if (relid != tab->relid) + LockRelationOid(relid, AccessExclusiveLock); ATPostAlterTypeParse(oldId, relid, InvalidOid, (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); @@ -15504,6 +15507,15 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) Oid relid; relid = StatisticsGetRelation(oldId, false); + + /* + * We use ShareUpdateExclusiveLock to lock the relation for which we + * will rebuild statistics, aligning with the lock level used in + * CreateStatistics and RemoveStatisticsById. + */ + if (relid != tab->relid) + LockRelationOid(relid, ShareUpdateExclusiveLock); + ATPostAlterTypeParse(oldId, relid, InvalidOid, (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 750efc042d8..cf2235328f1 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4750,6 +4750,14 @@ create table attbl(a int); create table atref(b attbl check ((b).a is not null)); alter table attbl alter column a type numeric; -- someday this should work ERROR: cannot alter table "attbl" because column "atref.b" uses its row type +alter table atref drop constraint atref_b_check; +create statistics atref_stat ON ((b).a is not null) from atref; +alter table attbl alter column a type numeric; --error +ERROR: cannot alter table "attbl" because column "atref.b" uses its row type +drop statistics atref_stat; +create index atref_idx on atref (((b).a)); +alter table attbl alter column a type numeric; --error +ERROR: cannot alter table "attbl" because column "atref.b" uses its row type drop table attbl, atref; /* End test case for bug #18970 */ -- Test that ALTER TABLE rewrite preserves a clustered index diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out index 16de30ab191..373a37b3838 100644 --- a/src/test/regress/expected/generated_stored.out +++ b/src/test/regress/expected/generated_stored.out @@ -1313,6 +1313,18 @@ CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') STORED, c te CREATE TABLE gtest31_2 (x int, y gtest31_1); ALTER TABLE gtest31_1 ALTER COLUMN b TYPE varchar; -- fails ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type +-- bug #18970, https://postgr.es/m/18970-a7d1cfe1f8d5d8d9@postgresql.org +ALTER TABLE gtest31_2 ADD CONSTRAINT cc CHECK ((y).b IS NOT NULL); +ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello'); --error +ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type +ALTER TABLE gtest31_2 DROP CONSTRAINT cc; +CREATE STATISTICS gtest31_2_stat ON ((y).b is not null) FROM gtest31_2; +ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello'); --error +ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type +DROP STATISTICS gtest31_2_stat; +CREATE INDEX gtest31_2_y_idx ON gtest31_2(((y).b)); +ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello'); --error +ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type DROP TABLE gtest31_1, gtest31_2; -- Check it for a partitioned table, too CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') STORED, c text) PARTITION BY LIST (a); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 41cff198e18..fcf522eb493 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -3074,6 +3074,14 @@ drop table attbl, atref; create table attbl(a int); create table atref(b attbl check ((b).a is not null)); alter table attbl alter column a type numeric; -- someday this should work + +alter table atref drop constraint atref_b_check; +create statistics atref_stat ON ((b).a is not null) from atref; +alter table attbl alter column a type numeric; --error +drop statistics atref_stat; + +create index atref_idx on atref (((b).a)); +alter table attbl alter column a type numeric; --error drop table attbl, atref; /* End test case for bug #18970 */ diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql index 4ec155f2da9..16ce1142058 100644 --- a/src/test/regress/sql/generated_stored.sql +++ b/src/test/regress/sql/generated_stored.sql @@ -595,6 +595,19 @@ ALTER TABLE gtest30_1 ALTER COLUMN b DROP EXPRESSION; -- error CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') STORED, c text); CREATE TABLE gtest31_2 (x int, y gtest31_1); ALTER TABLE gtest31_1 ALTER COLUMN b TYPE varchar; -- fails + +-- bug #18970, https://postgr.es/m/18970-a7d1cfe1f8d5d8d9@postgresql.org +ALTER TABLE gtest31_2 ADD CONSTRAINT cc CHECK ((y).b IS NOT NULL); +ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello'); --error +ALTER TABLE gtest31_2 DROP CONSTRAINT cc; + +CREATE STATISTICS gtest31_2_stat ON ((y).b is not null) FROM gtest31_2; +ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello'); --error +DROP STATISTICS gtest31_2_stat; + +CREATE INDEX gtest31_2_y_idx ON gtest31_2(((y).b)); +ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello'); --error + DROP TABLE gtest31_1, gtest31_2; -- Check it for a partitioned table, too -- 2.34.1