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

Commit e902f81

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 f062e43 commit e902f81

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
@@ -12966,6 +12966,14 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1296612966
Oid relid;
1296712967

1296812968
relid = IndexGetRelation(oldId, false);
12969+
12970+
/*
12971+
* As above, make sure we have lock on the index's table if it's not
12972+
* the same table.
12973+
*/
12974+
if (relid != tab->relid)
12975+
LockRelationOid(relid, AccessExclusiveLock);
12976+
1296912977
ATPostAlterTypeParse(oldId, relid, InvalidOid,
1297012978
(char *) lfirst(def_item),
1297112979
wqueue, lockmode, tab->rewrite);
@@ -12982,6 +12990,20 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1298212990
Oid relid;
1298312991

1298412992
relid = StatisticsGetRelation(oldId, false);
12993+
12994+
/*
12995+
* As above, make sure we have lock on the statistics object's table
12996+
* if it's not the same table. However, we take
12997+
* ShareUpdateExclusiveLock here, aligning with the lock level used in
12998+
* CreateStatistics and RemoveStatisticsById.
12999+
*
13000+
* CAUTION: this should be done after all cases that grab
13001+
* AccessExclusiveLock, else we risk causing deadlock due to needing
13002+
* to promote our table lock.
13003+
*/
13004+
if (relid != tab->relid)
13005+
LockRelationOid(relid, ShareUpdateExclusiveLock);
13006+
1298513007
ATPostAlterTypeParse(oldId, relid, InvalidOid,
1298613008
(char *) lfirst(def_item),
1298713009
wqueue, lockmode, tab->rewr 10000 ite);

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4587,6 +4587,14 @@ create table attbl(a int);
45874587
create table atref(b attbl check ((b).a is not null));
45884588
alter table attbl alter column a type numeric; -- someday this should work
45894589
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4590+
alter table atref drop constraint atref_b_check;
4591+
create statistics atref_stat on ((b).a is not null) from atref;
4592+
alter table attbl alter column a type numeric; -- someday this should work
4593+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4594+
drop statistics atref_stat;
4595+
create index atref_idx on atref (((b).a));
4596+
alter table attbl alter column a type numeric; -- someday this should work
4597+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
45904598
drop table attbl, atref;
45914599
/* End test case for bug #18970 */
45924600
-- 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
@@ -3027,6 +3027,14 @@ drop table attbl, atref;
30273027
create table attbl(a int);
30283028
create table atref(b attbl check ((b).a is not null));
30293029
alter table attbl alter column a type numeric; -- someday this should work
3030+
alter table atref drop constraint atref_b_check;
3031+
3032+
create statistics atref_stat on ((b).a is not null) from atref;
3033+
alter table attbl alter column a type numeric; -- someday this should work
3034+
drop statistics atref_stat;
3035+
3036+
create index atref_idx on atref (((b).a));
3037+
alter table attbl alter column a type numeric; -- someday this should work
30303038
drop table attbl, atref;
30313039

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

0 commit comments

Comments
 (0)
0