From 3ce3fb2f7dc66fef67c8184b96245d74372b729e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 19 Oct 2021 19:08:45 -0300 Subject: Ensure correct lock level is used in ALTER ... RENAME MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 1b5d797cd4f7 intended to relax the lock level used to rename indexes, but inadvertently allowed *any* relation to be renamed with a lowered lock level, as long as the command is spelled ALTER INDEX. That's undesirable for other relation types, so retry the operation with the higher lock if the relation turns out not to be an index. After this fix, ALTER INDEX RENAME will require access exclusive lock, which it didn't before. Author: Nathan Bossart Author: Álvaro Herrera Reported-by: Onder Kalaci Discussion: https://postgr.es/m/PH0PR21MB1328189E2821CDEC646F8178D8AE9@PH0PR21MB1328.namprd21.prod.outlook.com --- src/backend/commands/tablecmds.c | 62 +++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 14 deletions(-) (limited to 'src/backend/commands/tablecmds.c') diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 99b70d223d9..0d34f7007e1 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3733,7 +3733,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; @@ -3743,24 +3743,48 @@ RenameRelation(RenameStmt *stmt) * end of transaction. * * Lock level used here should match RenameRelationInternal, to avoid lock - * escalation. + * escalation. However, because ALTER INDEX can be used with any relation + * type, we mustn't believe without verification. */ - 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; + char relkind; + 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. We don't bother + * if we already acquired AccessExclusiveLock with an index, however. + */ + relkind = get_rel_relkind(relid); + obj_is_index = (relkind == RELKIND_INDEX || + relkind == RELKIND_PARTITIONED_INDEX); + if (obj_is_index || 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); @@ -3808,6 +3832,16 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo errmsg("relation \"%s\" already exists", newrelname))); + /* + * RenameRelation is careful not to believe the caller's idea of the + * relation kind being handled. We don't have to worry about this, but + * let's not be totally oblivious to it. We can process an index as + * not-an-index, but not the other way around. + */ + Assert(!is_index || + is_index == (targetrelation->rd_rel->relkind == RELKIND_INDEX || + targetrelation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)); + /* * Update pg_class tuple with new relname. (Scribbling on reltup is OK * because it's a copy...) -- cgit v1.2.3