From ebd8fc7e47fdad6adb68aad341d95c541d7325c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Thu, 9 Jan 2025 14:17:12 +0100 Subject: [PATCH] Simplify signature of RewriteTable This function doesn't need the lockmode to be passed: it was being used to lock the new heap, but that's bogus, because the only caller has already obtained the appropriate lock on the new heap (which is unimportant anyway, because the relation's creation is not yet committed and so no other session can see it). Noticed while reviewed Antonin Houska's patch to add VACUUM FULL CONCURRENTLY. --- src/backend/commands/tablecmds.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4181c110eb7..54575fcd287 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -433,7 +433,7 @@ static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab, static void ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, AlterTableUtilityContext *context); -static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode); +static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap); static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel); static void ATSimplePermissions(AlterTableType cmdtype, Relation rel, int allowed_targets); static void ATSimpleRecursion(List **wqueue, Relation rel, @@ -5901,7 +5901,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, * modifications, and test the current data within the table * against new constraints generated by ALTER TABLE commands. */ - ATRewriteTable(tab, OIDNewHeap, lockmode); + ATRewriteTable(tab, OIDNewHeap); /* * Swap the physical files of the old and new heaps, then rebuild @@ -5934,7 +5934,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, */ if (tab->constraints != NIL || tab->verify_new_notnull || tab->partition_constraint != NULL) - ATRewriteTable(tab, InvalidOid, lockmode); + ATRewriteTable(tab, InvalidOid); /* * If we had SET TABLESPACE but no reason to reconstruct tuples, @@ -6033,10 +6033,11 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, /* * ATRewriteTable: scan or rewrite one table * - * OIDNewHeap is InvalidOid if we don't need to rewrite + * A rewrite is requested by passing a valid OIDNewHeap; in that case, caller + * must already hold AccessExclusiveLock on it. */ static void -ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) +ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) { Relation oldrel; Relation newrel; @@ -6061,7 +6062,11 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) newTupDesc = RelationGetDescr(oldrel); /* includes all mods */ if (OidIsValid(OIDNewHeap)) - newrel = table_open(OIDNewHeap, lockmode); + { + Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock, + false)); + newrel = table_open(OIDNewHeap, NoLock); + } else newrel = NULL;