From c7db185e7c75f197a55d7e73b898b3b0858796cc Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Thu, 7 Oct 2021 00:36:06 +0000 Subject: [PATCH v2 1/1] Ensure correct lock level is used for rename statements. We allow mismatching statement and object types for some rename statements, but that can cause the wrong lock level to be used. When that happens, retry the relation lookup with the correct lock level. --- src/backend/commands/tablecmds.c | 45 ++++++++++++++++++++++--------- src/test/regress/expected/alter_table.out | 6 +++++ src/test/regress/sql/alter_table.sql | 7 +++++ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index ff97b618e6..d42ed13344 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3735,7 +3735,7 @@ RenameConstraint(RenameStmt *stmt) ObjectAddress RenameRelation(RenameStmt *stmt) { - bool is_index = stmt->renameType == OBJECT_INDEX; + bool is_index_stmt = stmt->renameType == OBJECT_INDEX; Oid relid; ObjectAddress address; @@ -3747,22 +3747,41 @@ RenameRelation(RenameStmt *stmt) * Lock level used here should match RenameRelationInternal, to avoid lock * escalation. */ - relid = RangeVarGetRelidExtended(stmt->relation, - is_index ? ShareUpdateExclusiveLock : AccessExclusiveLock, - stmt->missing_ok ? RVR_MISSING_OK : 0, - RangeVarCallbackForAlterRelation, - (void *) stmt); - - if (!OidIsValid(relid)) + for (;;) { - ereport(NOTICE, - (errmsg("relation \"%s\" does not exist, skipping", - stmt->relation->relname))); - return InvalidObjectAddress; + LOCKMODE lockmode; + bool obj_is_index; + + lockmode = is_index_stmt ? ShareUpdateExclusiveLock : AccessExclusiveLock; + + relid = RangeVarGetRelidExtended(stmt->relation, lockmode, + stmt->missing_ok ? RVR_MISSING_OK : 0, + RangeVarCallbackForAlterRelation, + (void *) stmt); + + if (!OidIsValid(relid)) + { + ereport(NOTICE, + (errmsg("relation \"%s\" does not exist, skipping", + stmt->relation->relname))); + return InvalidObjectAddress; + } + + /* + * We allow mismatched statement and object types (e.g., ALTER INDEX to + * rename a table), but we might've used the wrong lock level. If that + * happens, retry with the correct lock level. + */ + obj_is_index = (get_rel_relkind(relid) == RELKIND_INDEX); + if (is_index_stmt == obj_is_index) + break; + + UnlockRelationOid(relid, lockmode); + is_index_stmt = obj_is_index; } /* Do the work */ - RenameRelationInternal(relid, stmt->newname, false, is_index); + RenameRelationInternal(relid, stmt->newname, false, is_index_stmt); ObjectAddressSet(address, RelationRelationId, relid); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 4bee0c1173..b39430f201 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -232,6 +232,12 @@ SET ROLE regress_alter_table_user1; ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied ERROR: must be owner of index onek_unique1 RESET ROLE; +-- rename statements with mismatching statement and object types +CREATE TABLE alter_idx_rename_test (a INT); +CREATE INDEX alter_idx_rename_test_idx ON alter_idx_rename_test (a); +ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2; +ALTER TABLE alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2; +DROP TABLE alter_idx_rename_test_2; -- renaming views CREATE VIEW attmp_view (unique1) AS SELECT unique1 FROM tenk1; ALTER TABLE attmp_view RENAME TO attmp_view_new; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index dc0200adcb..a56926f915 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -227,6 +227,13 @@ SET ROLE regress_alter_table_user1; ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied RESET ROLE; +-- rename statements with mismatching statement and object types +CREATE TABLE alter_idx_rename_test (a INT); +CREATE INDEX alter_idx_rename_test_idx ON alter_idx_rename_test (a); +ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2; +ALTER TABLE alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2; +DROP TABLE alter_idx_rename_test_2; + -- renaming views CREATE VIEW attmp_view (unique1) AS SELECT unique1 FROM tenk1; ALTER TABLE attmp_view RENAME TO attmp_view_new; -- 2.16.6