Obtain required table lock during cross-table updates, redux. · postgres/postgres@a10f21e · GitHub | Latest TMZ Celebrity News & Gossip | Watch TMZ Live
Skip to content

Commit a10f21e

Browse files
tglsfdctglsfdc
committed
Obtain required table lock during cross-table updates, redux.
Commits 8319e5c et al missed the fact that ATPostAlterTypeCleanup contains three calls to ATPostAlterTypeParse, and the other two also need protection against passing a relid that we don't yet have lock on. Add similar logic to those code paths, and add some test cases demonstrating the need for it. In v18 and master, the test cases demonstrate that there's a behavioral discrepancy between stored generated columns and virtual generated columns: we disallow changing the expression of a stored column if it's used in any composite-type columns, but not that of a virtual column. Since the expression isn't actually relevant to either sort of composite-type usage, this prohibition seems unnecessary; but changing it is a matter for separate discussion. For now we are just documenting the existing behavior. Reported-by: jian he <jian.universality@gmail.com> Author: jian he <jian.universality@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: CACJufxGKJtGNRRSXfwMW9SqVOPEMdP17BJ7DsBf=tNsv9pWU9g@mail.gmail.com Backpatch-through: 13
1 parent a604aff commit a10f21e

File tree

7 files changed

+85
-0
lines changed

7 files changed

+85
-0
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15487,6 +15487,14 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1548715487
Oid relid;
1548815488

1548915489
relid = IndexGetRelation(oldId, false);
15490+
15491+
/*
15492+
* As above, make sure we have lock on the index's table if it's not
15493+
* the same table.
15494+
*/
15495+
if (relid != tab->relid)
15496+
LockRelationOid(relid, AccessExclusiveLock);
15497+
1549015498
ATPostAlterTypeParse(oldId, relid, InvalidOid,
1549115499
(char *) lfirst(def_item),
1549215500
wqueue, lockmode, tab->rewrite);
@@ -15503,6 +15511,20 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1550315511
Oid relid;
1550415512

1550515513
relid = StatisticsGetRelation(oldId, false);
15514+
15515+
/*
15516+
* As above, make sure we have lock on the statistics object's table
15517+
* if it's not the same table. However, we take
15518+
* ShareUpdateExclusiveLock here, aligning with the lock level used in
15519+
* CreateStatistics and RemoveStatisticsById.
15520+
*
15521+
* CAUTION: this should be done after all cases that grab
15522+
* AccessExclusiveLock, else we risk causing deadlock due to needing
15523+
* to promote our table lock.
15524+
*/
15525+
if (relid != tab->relid)
15526+
LockRelationOid(relid, ShareUpdateExclusiveLock);
15527+
1550615528
ATPostAlterTypeParse(oldId, relid, InvalidOid,
1550715529
(char *) lfirst(def_item),
1550815530
wqueue, lockmode, tab->rewrite);

‎src/test/regress/expected/alter_table.out

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4750,6 +4750,14 @@ create table attbl(a int);
47504750
create table atref(b attbl check ((b).a is not null));
47514751
alter table attbl alter column a type numeric; -- someday this should work
47524752
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4753+
alter table atref drop constraint atref_b_check;
4754+
create statistics atref_stat on ((b).a is not null) from atref;
4755+
alter table attbl alter column a type numeric; -- someday this should work
4756+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4757+
drop statistics atref_stat;
4758+
create index atref_idx on atref (((b).a));
4759+
alter table attbl alter column a type numeric; -- someday this should work
4760+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
47534761
drop table attbl, atref;
47544762
/* End test case for bug #18970 */
47554763
-- Test that ALTER TABLE rewrite preserves a clustered index

‎src/test/regress/expected/generated_stored.out

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,6 +1313,18 @@ CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') STORED, c te
13131313
CREATE TABLE gtest31_2 (x int, y gtest31_1);
13141314
ALTER TABLE gtest31_1 ALTER COLUMN b TYPE varchar; -- fails
13151315
ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type
1316+
-- bug #18970: these cases are unsupported, but make sure they fail cleanly
1317+
ALTER TABLE gtest31_2 ADD CONSTRAINT cc CHECK ((y).b IS NOT NULL);
1318+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello1');
1319+
ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type
1320+
ALTER TABLE gtest31_2 DROP CONSTRAINT cc;
1321+
CREATE STATISTICS gtest31_2_stat ON ((y).b is not null) FROM gtest31_2;
1322+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello2');
1323+
ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type
1324+
DROP STATISTICS gtest31_2_stat;
1325+
CREATE INDEX gtest31_2_y_idx ON gtest31_2(((y).b));
1326+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello3');
1327+
ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type
13161328
DROP TABLE gtest31_1, gtest31_2;
13171329
-- Check it for a partitioned table, too
13181330
CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') STORED, c text) PARTITION BY LIST (a);

‎src/test/regress/expected/generated_virtual.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,15 @@ CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') VIRTUAL, c t
12831283
CREATE TABLE gtest31_2 (x int, y gtest31_1);
12841284
ALTER TABLE gtest31_1 ALTER COLUMN b TYPE varchar; -- fails
12851285
ERROR: cannot alter table "gtest31_1" because column "gtest31_2.y" uses its row type
1286+
-- bug #18970
1287+
ALTER TABLE gtest31_2 ADD CONSTRAINT cc CHECK ((y).b IS NOT NULL);
1288+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello1');
1289+
ALTER TABLE gtest31_2 DROP CONSTRAINT cc;
1290+
CREATE STATISTICS gtest31_2_stat ON ((y).b is not null) FROM gtest31_2;
1291+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello2');
1292+
DROP STATISTICS gtest31_2_stat;
1293+
CREATE INDEX gtest31_2_y_idx ON gtest31_2(((y).b));
1294+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello3');
12861295
DROP TABLE gtest31_1, gtest31_2;
12871296
-- Check it for a partitioned table, too
12881297
CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') VIRTUAL, c text) PARTITION BY LIST (a);

‎src/test/regress/sql/alter_table.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3074,6 +3074,14 @@ drop table attbl, atref;
30743074
create table attbl(a int);
30753075
create table atref(b attbl check ((b).a is not null));
30763076
alter table attbl alter column a type numeric; -- someday this should work
3077+
alter table atref drop constraint atref_b_check;
3078+
3079+
create statistics atref_stat on ((b).a is not null) from atref;
3080+
alter table attbl alter column a type numeric; -- someday this should work
3081+
drop statistics atref_stat;
3082+
3083+
create index atref_idx on atref (((b).a));
3084+
alter table attbl alter column a type numeric; -- someday this should work
30773085
drop table attbl, atref;
30783086

30793087
/* End test case for bug #18970 */

‎src/test/regress/sql/generated_stored.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,19 @@ ALTER TABLE gtest30_1 ALTER COLUMN b DROP EXPRESSION; -- error
595595
CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') STORED, c text);
596596
CREATE TABLE gtest31_2 (x int, y gtest31_1);
597597
ALTER TABLE gtest31_1 ALTER COLUMN b TYPE varchar; -- fails
598+
599+
-- bug #18970: these cases are unsupported, but make sure they fail cleanly
600+
ALTER TABLE gtest31_2 ADD CONSTRAINT cc CHECK ((y).b IS NOT NULL);
601+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello1');
602+
ALTER TABLE gtest31_2 DROP CONSTRAINT cc;
603+
604+
CREATE STATISTICS gtest31_2_stat ON ((y).b is not null) FROM gtest31_2;
605+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello2');
606+
DROP STATISTICS gtest31_2_stat;
607+
608+
CREATE INDEX gtest31_2_y_idx ON gtest31_2(((y).b));
609+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello3');
610+
598611
DROP TABLE gtest31_1, gtest31_2;
599612

600613
-- Check it for a partitioned table, too

‎src/test/regress/sql/generated_virtual.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,19 @@ ALTER TABLE gtest30_1 ALTER COLUMN b DROP EXPRESSION; -- error
646646
CREATE TABLE gtest31_1 (a int, b text GENERATED ALWAYS AS ('hello') VIRTUAL, c text);
647647
CREATE TABLE gtest31_2 (x int, y gtest31_1);
648648
ALTER TABLE gtest31_1 ALTER COLUMN b TYPE varchar; -- fails
649+
650+
-- bug #18970
651+
ALTER TABLE gtest31_2 ADD CONSTRAINT cc CHECK ((y).b IS NOT NULL);
652+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello1');
653+
ALTER TABLE gtest31_2 DROP CONSTRAINT cc;
654+
655+
CREATE STATISTICS gtest31_2_stat ON ((y).b is not null) FROM gtest31_2;
656+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello2');
657+
DROP STATISTICS gtest31_2_stat;
658+
659+
CREATE INDEX gtest31_2_y_idx ON gtest31_2(((y).b));
660+
ALTER TABLE gtest31_1 ALTER COLUMN b SET EXPRESSION AS ('hello3');
661+
649662
DROP TABLE gtest31_1, gtest31_2;
650663

651664
-- Check it for a partitioned table, too

0 commit comments

Comments
 (0)

TMZ Celebrity News – Breaking Stories, Videos & Gossip

Looking for the latest TMZ celebrity news? You've come to the right place. From shocking Hollywood scandals to exclusive videos, TMZ delivers it all in real time.

Whether it’s a red carpet slip-up, a viral paparazzi moment, or a legal drama involving your favorite stars, TMZ news is always first to break the story. Stay in the loop with daily updates, insider tips, and jaw-dropping photos.

🎥 Watch TMZ Live

TMZ Live brings you daily celebrity news and interviews straight from the TMZ newsroom. Don’t miss a beat—watch now and see what’s trending in Hollywood.