Adjust comments of CheckRelationTableSpaceMove() and SetRelationTableSpace()

michaelpq · michaelpq · commit 24843297a96d · 2021-01-29T13:59:18.000+09:00
4c9c359, that introduced those two functions, has been overoptimistic on the point that only ShareUpdateExclusiveLock would be required when moving a relation to a new tablespace. AccessExclusiveLock is a requirement, but ShareUpdateExclusiveLock may be used under specific conditions like REINDEX CONCURRENTLY where waits on past transactions make the operation safe even with a lower-level lock. The current code does only the former, so update the existing comments to reflect that. Once a REINDEX (TABLESPACE) is introduced, those comments would require an extra refresh to mention their new use case. While on it, fix an incorrect variable name. Per discussion with Álvaro Herrera. Discussion: https://postgr.es/m/20210127140741.GA14174@alvherre.pgsql
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
@@ -3041,11 +3041,11 @@ SetRelationHasSubclass(Oid relationId, bool relhassubclass)
  * CheckRelationTableSpaceMove
  *		Check if relation can be moved to new tablespace.
  *
- * NOTE: Caller must be holding an appropriate lock on the relation.
- * ShareUpdateExclusiveLock is sufficient.
+ * NOTE: The caller must hold AccessExclusiveLock on the relation.
  *
- * Returns true if the relation can be moved to the new tablespace;
- * false otherwise.
+ * Returns true if the relation can be moved to the new tablespace; raises
+ * an error if it is not possible to do the move; returns false if the move
+ * would have no effect.
  */
 bool
 CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)
@@ -3094,11 +3094,10 @@ CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)
  *		Set new reltablespace and relfilenode in pg_class entry.
  *
  * newTableSpaceId is the new tablespace for the relation, and
- * newRelFileNode its new filenode.  If newrelfilenode is InvalidOid,
+ * newRelFileNode its new filenode.  If newRelFileNode is InvalidOid,
  * this field is not updated.
  *
- * NOTE: Caller must be holding an appropriate lock on the relation.
- * ShareUpdateExclusiveLock is sufficient.
+ * NOTE: The caller must hold AccessExclusiveLock on the relation.
  *
  * The caller of this routine had better check if a relation can be
  * moved to this new tablespace by calling CheckRelationTableSpaceMove()

Original file line number	Diff line number	Diff line change
`@@ -3041,11 +3041,11 @@ SetRelationHasSubclass(Oid relationId, bool relhassubclass)`
`3041`	`3041`	`* CheckRelationTableSpaceMove`
`3042`	`3042`	`* Check if relation can be moved to new tablespace.`
`3043`	`3043`	`*`
`3044`		`- * NOTE: Caller must be holding an appropriate lock on the relation.`
`3045`		`- * ShareUpdateExclusiveLock is sufficient.`
	`3044`	`+ * NOTE: The caller must hold AccessExclusiveLock on the relation.`
`3046`	`3045`	`*`
`3047`		`- * Returns true if the relation can be moved to the new tablespace;`
`3048`		`- * false otherwise.`
	`3046`	`+ * Returns true if the relation can be moved to the new tablespace; raises`
	`3047`	`+ * an error if it is not possible to do the move; returns false if the move`
	`3048`	`+ * would have no effect.`
`3049`	`3049`	`*/`
`3050`	`3050`	`bool`
`3051`	`3051`	`CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)`
`@@ -3094,11 +3094,10 @@ CheckRelationTableSpaceMove(Relation rel, Oid newTableSpaceId)`
`3094`	`3094`	`* Set new reltablespace and relfilenode in pg_class entry.`
`3095`	`3095`	`*`
`3096`	`3096`	`* newTableSpaceId is the new tablespace for the relation, and`
`3097`		`- * newRelFileNode its new filenode. If newrelfilenode is InvalidOid,`
	`3097`	`+ * newRelFileNode its new filenode. If newRelFileNode is InvalidOid,`
`3098`	`3098`	`* this field is not updated.`
`3099`	`3099`	`*`
`3100`		`- * NOTE: Caller must be holding an appropriate lock on the relation.`
`3101`		`- * ShareUpdateExclusiveLock is sufficient.`
	`3100`	`+ * NOTE: The caller must hold AccessExclusiveLock on the relation.`
`3102`	`3101`	`*`
`3103`	`3102`	`* The caller of this routine had better check if a relation can be`
`3104`	`3103`	`* moved to this new tablespace by calling CheckRelationTableSpaceMove()`