8000 Obtain required table lock during cross-table updates, redux. · postgres/postgres@d36980b · GitHub
[go: up one dir, main page]
Skip to content

Commit d36980b

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 db51bdf commit d36980b

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

‎src/backend/commands/tablecmds.c

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

1356313563
relid = IndexGetRelation(oldId, false);
13564+
13565+
/*
13566+
* As above, make sure we have lock on the index's table if it's not
13567+
* the same table.
13568+
*/
13569+
if (relid != tab->relid)
13570+
LockRelationOid(relid, AccessExclusiveLock);
13571+
1356413572
ATPostAlterTypeParse(oldId, relid, InvalidOid,
1356513573
(char *) lfirst(def_item),
1356613574
wqueue, lockmode, tab->rewrite);
@@ -13577,6 +13585,20 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1357713585
Oid relid;
1357813586

1357913587
relid = StatisticsGetRelation(oldId, false);
13588+
13589+
/*
13590+
* As above, make sure we have lock on the statistics object's table
13591+
* if it's not the same table. However, we take
13592+
* ShareUpdateExclusiveLock here, aligning with the lock level used in
13593+
* CreateStatistics and RemoveStatisticsById.
13594+
*
13595+
* CAUTION: this should be done after all cases that grab
13596+
* AccessExclusiveLock, else we risk causing deadlock due to needing
13597+
* to promote our table lock.
13598+
*/
13599+
if (relid != tab->relid)
13600+
LockRelationOid(relid, ShareUpdateExclusiveLock);
13601+
1358013602
ATPostAlterTypeParse(oldId, relid, InvalidOid,
1358113603
(char *) lfirst(def_item),
1358213604
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
@@ -4588,6 +4588,14 @@ create table attbl(a int);
45884588
create table atref(b attbl check ((b).a is not null));
45894589
alter table attbl alter column a type numeric; -- someday this should work
45904590
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4591+
alter table atref drop constraint atref_b_check;
4592+
create statistics atref_stat on ((b).a is not null) from atref;
4593+
alter table attbl alter column a type numeric; -- someday this should work
4594+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4595+
drop statistics atref_stat;
4596+
create index atref_idx on atref (((b).a));
4597+
alter table attbl alter column a type numeric; -- someday this should work
4598+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
45914599
drop table attbl, atref;
45924600
/* End test case for bug #18970 */
45934601
-- Test that ALTER TABLE rewrite preserves a clustered index

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3025,6 +3025,14 @@ drop table attbl, atref;
30253025
create table attbl(a int);
30263026
create table atref(b attbl check ((b).a is not null));
30273027
alter table attbl alter column a type numeric; -- someday this should work
3028+
alter table atref drop constraint atref_b_check;
3029+
3030+
create statistics atref_stat on ((b).a is not null) from atref;
3031+
alter table attbl alter column a type numeric; -- someday this should work
3032+
drop statistics atref_stat;
3033+
3034+
create index atref_idx on atref (((b).a));
3035+
alter table attbl alter column a type numeric; -- someday this should work
30283036
drop table attbl, atref;
30293037

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

0 commit comments

Comments
 (0)
0