Obtain required table lock during cross-table updates, redux.

Commits 8319e5cb5 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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a3c7e26190b3c927a8918aec4547ff10f482dcee..015e11af67bd8287380350cf04fc1581640cd691 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12966,6 +12966,14 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
                Oid                     relid;
 
                relid = IndexGetRelation(oldId, false);
+
+               /*
+                * As above, make sure we have lock on the index's table if it's not
+                * the same table.
+                */
+               if (relid != tab->relid)
+                       LockRelationOid(relid, AccessExclusiveLock);
+
                ATPostAlterTypeParse(oldId, relid, InvalidOid,
                                                         (char *) lfirst(def_item),
                                                         wqueue, lockmode, tab->rewrite);
@@ -12982,6 +12990,20 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
                Oid                     relid;
 
                relid = StatisticsGetRelation(oldId, false);
+
+               /*
+                * As above, make sure we have lock on the statistics object's table
+                * if it's not the same table.  However, we take
+                * ShareUpdateExclusiveLock here, aligning with the lock level used in
+                * CreateStatistics and RemoveStatisticsById.
+                *
+                * CAUTION: this should be done after all cases that grab
+                * AccessExclusiveLock, else we risk causing deadlock due to needing
+                * to promote our table lock.
+                */
+               if (relid != tab->relid)
+                       LockRelationOid(relid, ShareUpdateExclusiveLock);
+
                ATPostAlterTypeParse(oldId, relid, InvalidOid,
                                                         (char *) lfirst(def_item),
                                                         wqueue, lockmode, tab->rewrite);

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 20b7b3c651e97304a73ef4ac4d3d3dc4686f9d9d..3305c6ed24a42102f7cd0b3b8e9f34de6196672f 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4587,6 +4587,14 @@ create table attbl(a int);
 create table atref(b attbl check ((b).a is not null));
 alter table attbl alter column a type numeric;  -- someday this should work
 ERROR:  cannot alter table "attbl" because column "atref.b" uses its row type
+alter table atref drop constraint atref_b_check;
+create statistics atref_stat on ((b).a is not null) from atref;
+alter table attbl alter column a type numeric;  -- someday this should work
+ERROR:  cannot alter table "attbl" because column "atref.b" uses its row type
+drop statistics atref_stat;
+create index atref_idx on atref (((b).a));
+alter table attbl alter column a type numeric;  -- someday this should work
+ERROR:  cannot alter table "attbl" because column "atref.b" uses its row type
 drop table attbl, atref;
 /* End test case for bug #18970 */
 -- Test that ALTER TABLE rewrite preserves a clustered index

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index fc9396472d17305be491bf93a67192260a4488c6..b7aa3ee179f65b9cae7d2855e454c13321a0617f 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -3027,6 +3027,14 @@ drop table attbl, atref;
 create table attbl(a int);
 create table atref(b attbl check ((b).a is not null));
 alter table attbl alter column a type numeric;  -- someday this should work
+alter table atref drop constraint atref_b_check;
+
+create statistics atref_stat on ((b).a is not null) from atref;
+alter table attbl alter column a type numeric;  -- someday this should work
+drop statistics atref_stat;
+
+create index atref_idx on atref (((b).a));
+alter table attbl alter column a type numeric;  -- someday this should work
 drop table attbl, atref;
 
 /* End test case for bug #18970 */