8000 Obtain required table lock during cross-table constraint updates. · postgres/postgres@13f1e9f · GitHub
[go: up one dir, main page]
Skip to content < 65F3 script type="application/json" data-target="react-partial.embeddedData">{"props":{"docsUrl":"https://docs.github.com/get-started/accessibility/keyboard-shortcuts"}}

Commit 13f1e9f

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 bf377c2 commit 13f1e9f

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
@@ -12560,9 +12560,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1256012560
/*
1256112561
* Re-parse the index and constraint definitions, and attach them to the
1256212562
* appropriate work queue entries. We do this before dropping because in
12563-
* the case of a FOREIGN KEY constraint, we might not yet have exclusive
12564-
* lock on the table the constraint is attached to, and we need to get
12565-
* that before reparsing/dropping.
12563+
* the case of a constraint on another table, we might not yet have
12564+
* exclusive lock on the table the constraint is attached to, and we need
12565+
* to get that before reparsing/dropping. (That's possible at least for
12566+
* FOREIGN KEY, CHECK, and EXCLUSION constraints; in non-FK cases it
12567+
* requires a dependency on the target table's composite type in the other
12568+
* table's constraint expressions.)
1256612569
*
1256712570
* We can't rely on the output of deparsing to tell us which relation to
1256812571
* operate on, because concurrent activity might have made the name
@@ -12578,7 +12581,6 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1257812581
Form_pg_constraint con;
1257912582
Oid relid;
1258012583
Oid confrelid;
12581-
char contype;
1258212584
bool conislocal;
1258312585

1258412586
tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(oldId));
@@ -12595,7 +12597,6 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1259512597
elog(ERROR, "could not identify relation associated with constraint %u", oldId);
1259612598
}
1259712599
confrelid = con->confrelid;
12598-
contype = con->contype;
1259912600
conislocal = con->conislocal;
1260012601
ReleaseSysCache(tup);
1260112602

@@ -12612,12 +12613,12 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
1261212613
continue;
1261312614

1261412615
/*
12615-
* When rebuilding an FK constraint that references the table we're
12616-
* modifying, we might not yet have any lock on the FK's table, so get
12617-
* one now. We'll need AccessExclusiveLock for the DROP CONSTRAINT
12618-
* step, so there's no value in asking for anything weaker.
12616+
* When rebuilding another table's constraint that references the
12617+
* table we're modifying, we might not yet have any lock on the other
12618+
* table, so get one now. We'll need AccessExclusiveLock for the DROP
12619+
* CONSTRAINT step, so there's no value in asking for anything weaker.
1261912620
*/
12620-
if (relid != tab->relid && contype == CONSTRAINT_FOREIGN)
12621+
if (relid != tab->relid)
1262112622
LockRelationOid(relid, AccessExclusiveLock);
1262212623

1262312624
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
@@ -4508,6 +4508,13 @@ create trigger xtrig
45084508
update bar1 set a = a + 1;
45094509
INFO: a=1, b=1
45104510
/* End test case for bug #16242 */
4511+
/* Test case for bug #18970 */
4512+
create table attbl(a int);
4513+
create table atref(b attbl check ((b).a is not null));
4514+
alter table attbl alter column a type numeric; -- someday this should work
4515+
ERROR: cannot alter table "attbl" because column "atref.b" uses its row type
4516+
drop table attbl, atref;
4517+
/* End test case for bug #18970 */
45114518
-- Test that ALTER TABLE rewrite preserves a clustered index
45124519
-- for normal indexes and indexes on constraints.
45134520
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
@@ -2978,6 +2978,15 @@ update bar1 set a = a + 1;
29782978

29792979
/* End test case for bug #16242 */
29802980

2981+
/* Test case for bug #18970 */
2982+
2983+
create table attbl(a int);
2984+
create table atref(b attbl check ((b).a is not null));
2985+
alter table attbl alter column a type numeric; -- someday this should work
2986+
drop table attbl, atref;
2987+
2988+
/* End test case for bug #18970 */
2989+
29812990
-- Test that ALTER TABLE rewrite preserves a clustered index
29822991
-- for normal indexes and indexes on constraints.
29832992
create table alttype_cluster (a int);

0 commit comments

Comments
 (0)
0