Adjust signature of cluster_rel() and its subroutines

cluster_rel() receives the OID of the relation to process, which it
opens and locks; but then its subroutine copy_table_data() also receives
the relation OID and opens it by itself.  This is a bit wasteful.  It's
better to have cluster_rel() receive the relation already open, and pass
it down to its subroutines as necessary; then cluster_rel closes the rel
before returning.  This simplifies things.

But a better motivation to make this change is that a future command to
do logical-decoding-based "concurrent VACUUM FULL" will need to release
all locks on the relation (and possibly on the clustering index) at some
point.  Since it makes little sense to keep the relation reference
without the lock, the cluster_rel() function will also close it (and
the index).  With this arrangement, neither the function nor its
subroutines need open extra references, which, again, makes things simpler.

Author: Antonin Houska <[email protected]>
Discussion: https://postgr.es/m/82651.1720540558@antos

1 files changed, 4 insertions, 5 deletions

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2640228bef4..e6745e6145c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2218,15 +2218,14 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
 		{
 			ClusterParams cluster_params = {0};
 
-			/* close relation before vacuuming, but hold lock until commit */
-			relation_close(rel, NoLock);
-			rel = NULL;
-
 			if ((params->options & VACOPT_VERBOSE) != 0)
 				cluster_params.options |= CLUOPT_VERBOSE;
 
 			/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
-			cluster_rel(relid, InvalidOid, &cluster_params);
+			cluster_rel(rel, InvalidOid, &cluster_params);
+			/* cluster_rel closes the relation, but keeps lock */
+
+			rel = NULL;
 		}
 		else
 			table_relation_vacuum(rel, params, bstrategy);