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

Commit c15798c

Browse files
tglsfdctglsfdc
committed
Obtain required table lock during cross-table constraint updates.
Sometimes a table's constraint may depend on a column of another table, so that we have to update the constraint when changing the referenced column's type. We need to have lock on the constraint's table to do that. ATPostAlterTypeCleanup believed that this case was only possible for FOREIGN KEY constraints, but it's wrong at least for CHECK and EXCLUDE constraints; and in general, we'd probably need exclusive lock to alter any sort of constraint. So just remove the contype check and acquire lock for any other table. This prevents a "you don't have lock" assertion failure, though no ill effect is observed in production builds. We'll error out later anyway because we don't presently support physically altering column types within stored composite columns. But the catalog-munging is basically all there, so we may as well make that part work. Bug: #18970 Reported-by: Alexander Lakhin <exclusion@gmail.com> Diagnosed-by: jian he <jian.universality@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18970-a7d1cfe1f8d5d8d9@postgresql.org Backpatch-through: 13
1 parent 01f26f5 commit c15798c

File tree

3 files changed

+27
-10
lines changed

3 files changed

+27
-10
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13489,9 +13489,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1348913489
/*
1349013490
* Re-parse the index and constraint definitions, and attach them to the
1349113491
* appropriate work queue entries. We do this before dropping because in
13492-
* the case of a FOREIGN KEY constraint, we might not yet have exclusive
13493-
* lock on the table the constraint is attached to, and we need to get
13494-
* that before reparsing/dropping.
13492+
* the case of a constraint on another table, we might not yet have
13493+
* exclusive lock on the table the constraint is attached to, and we need
13494+
* to get that before reparsing/dropping. (That's possible at least for
13495+
* FOREIGN KEY, CHECK, and EXCLUSION constraints; in non-FK cases it
13496+
* requires a dependency on the target table's composite type in the other
13497+
* table's constraint expressions.)
1349513498
*
1349613499
* We can't rely on the output of deparsing to tell us which relation to
1349713500
* operate on, because concurrent activity might have made the name
@@ -13507,7 +13510,6 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1350713510
Form_pg_constraint con;
1350813511
Oid relid;
1350913512
Oid confrelid;
13510-
char contype;
1351113513
bool conislocal;
1351213514

1351313515
tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(oldId));
@@ -13524,7 +13526,6 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1352413526
elog(ERROR, "could not identify relation associated with constraint %u", oldId);
1352513527
}
1352613528
confrelid = con->confrelid;
13527-
contype = con->contype;
1352813529
conislocal = con->conislocal;
1352913530
ReleaseSysCache(tup);
1353013531

@@ -13541,12 +13542,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1354113542
continue;
1354213543

1354313544
/*
13544-
* When rebuilding an FK constraint that references the table we're
13545-
* modifying, we might not yet have any lock on the FK's table, so get
13546-
* one now. We'll need AccessExclusiveLock for the DROP CONSTRAINT
13547-
* step, so there's no value in asking for anything weaker.
13545+
* When rebuilding another table's constraint that references the
13546+
* table we're modifying, we might not yet have any lock on the other
13547+
* table, so get one now. We'll need AccessExclusiveLock for the DROP
13548+
* CONSTRAINT step, so there's no value in asking for anything weaker.
1354813549
*/
13549-
if (relid != tab->relid && contype == CONSTRAINT_FOREIGN)
13550+
if (relid != tab->relid)
1355013551
LockRelationOid(relid, AccessExclusiveLock);
1355113552

1355213553
ATPostAlterTypeParse(oldId, relid, confrelid,

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4583,6 +4583,13 @@ alter table attbl alter column p1 set data type bigint;
45834583
alter table atref alter column c1 set data type bigint;
45844584
drop table attbl, atref;
45854585
/* End test case for bug #17409 */
4586+
/* Test case for bug #18970 */
4587+
create table attbl(a int);
4588+
create table atref(b attbl check ((b).a is not null));
4589+
alter table attbl alter column a type numeric; -- someday this should work
4590+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4591+
drop table attbl, atref;
4592+
/* End test case for bug #18970 */
45864593
-- Test that ALTER TABLE rewrite preserves a clustered index
45874594
-- for normal indexes and indexes on constraints.
45884595
create table alttype_cluster (a int);

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3020,6 +3020,15 @@ drop table attbl, atref;
30203020

30213021
/* End test case for bug #17409 */
30223022

3023+
/* Test case for bug #18970 */
3024+
3025+
create table attbl(a int);
3026+
create table atref(b attbl check ((b).a is not null));
3027+
alter table attbl alter column a type numeric; -- someday this should work
3028+
drop table attbl, atref;
3029+
3030+
/* End test case for bug #18970 */
3031+
30233032
-- Test that ALTER TABLE rewrite preserves a clustered index
30243033
-- for normal indexes and indexes on constraints.
30253034
create table alttype_cluster (a int);

0 commit comments

Comments
 (0)
0