agora inbox for pgsql-hackers@postgresql.org  

help / color / mirror / Atom feed

[PATCH 2/4] Refactor and reuse set_rel_tablespace()
8+ messages / 2 participants
[nested] [flat]

* [PATCH 2/4] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 70 +++++++++++++++++++++-----------
 src/backend/commands/indexcmds.c | 35 ----------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9aa9fdf291..f652459d83 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3722,24 +3722,11 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		params->tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
 	if (set_tablespace &&
-			params->tablespaceOid != iRel->rd_rel->reltablespace)
+		set_rel_tablespace(indexId, params->tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
@@ -3747,14 +3734,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		 */
 		RelationDropStorage(iRel);
 
-		/* Update the pg_class row */
-		rd_rel->reltablespace = params->tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -4063,6 +4042,51 @@ reindex_relation(Oid relid, int flags, ReindexParams *params)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 4dd54079a2..efcfc2b844 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2942,41 +2942,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 56e403ac61..30cfd1877f 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -153,6 +153,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, ReindexParams *params);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--tsOsTdHNUZQcU9Ye
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0003-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespace.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v33 3/5] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 74 ++++++++++++++++++++------------
 src/backend/commands/indexcmds.c | 35 ---------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index cadc56733b..1eeb545307 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3607,7 +3607,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	Relation	iRel,
 				heapRelation;
 	Oid			heapId;
-	Oid			oldTablespaceOid;
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
@@ -3723,41 +3722,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
-	oldTablespaceOid = iRel->rd_rel->reltablespace;
 	if (set_tablespace &&
-		(tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+		set_rel_tablespace(indexId, tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
 		 * miss things.
 		 */
 		RelationDropStorage(iRel);
-
-		/* Update the pg_class row */
-		rd_rel->reltablespace = tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -4064,6 +4039,51 @@ reindex_relation(Oid relid, int flags, int options, Oid tablespaceOid)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 9300596385..f557600c43 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2938,41 +2938,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 178ddce63c..14af380c5f 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -149,6 +149,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, int options, Oid tablespaceOid);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--0MC1Z9bqFMhiUToQ
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v33-0004-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v29 4/7] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 74 ++++++++++++++++++++------------
 src/backend/commands/indexcmds.c | 39 +----------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 51 insertions(+), 64 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index bee68abbd2..82696edd41 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3437,7 +3437,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	Relation	iRel,
 				heapRelation;
 	Oid			heapId;
-	Oid			oldTablespaceOid;
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
@@ -3553,41 +3552,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
-	oldTablespaceOid = iRel->rd_rel->reltablespace;
 	if (set_tablespace &&
-		(tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+		set_rel_tablespace(indexId, tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
 		 * miss things.
 		 */
 		RelationDropStorage(iRel);
-
-		/* Update the pg_class row */
-		rd_rel->reltablespace = tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -3890,6 +3865,51 @@ reindex_relation(Oid relid, int flags, int options, Oid tablespaceOid)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 516feeb402..1cb9ba6d57 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2833,41 +2833,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
@@ -2941,7 +2906,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel, char *tablespace)
 		 * relations.
 		 */
 		if (partkind == RELKIND_PARTITIONED_INDEX)
-			set_rel_tablespace(partoid, tablespaceOid);
+			(void) set_rel_tablespace(partoid, tablespaceOid);
 		else if (partkind == RELKIND_PARTITIONED_TABLE)
 		{
 			Relation rel = table_open(partoid, ShareLock);
@@ -2952,7 +2917,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel, char *tablespace)
 			foreach (lc, indexIds)
 			{
 				Oid indexid = lfirst_oid(lc);
-				set_rel_tablespace(indexid, tablespaceOid);
+				(void) set_rel_tablespace(indexid, tablespaceOid);
 			}
 		}
 
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 6a667df355..a7745ba816 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -137,6 +137,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, int options, Oid tablespaceOid);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--k1lZvvs/B4yU6o8G
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v29-0005-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v34 5/8] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 73 ++++++++++++++++++++------------
 src/backend/commands/indexcmds.c | 35 ---------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 49 insertions(+), 61 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index aaac698eb0..8911959336 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3607,7 +3607,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	Relation	iRel,
 				heapRelation;
 	Oid			heapId;
-	Oid			oldTablespaceOid;
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
@@ -3722,26 +3721,11 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		options->tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
-	oldTablespaceOid = iRel->rd_rel->reltablespace;
 	if (set_tablespace &&
-		(options->tablespaceOid != oldTablespaceOid ||
-		(options->tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+		set_rel_tablespace(indexId, options->tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
@@ -3749,14 +3733,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		 */
 		RelationDropStorage(iRel);
 
-		/* Update the pg_class row */
-		rd_rel->reltablespace = options->tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -4064,6 +4040,51 @@ reindex_relation(Oid relid, int flags, ReindexOptions *options)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 849fecc861..2379d7d1a8 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2930,41 +2930,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 5db1d70064..9543ca9c61 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -150,6 +150,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, ReindexOptions *options);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--ucW7QTJbHsz5ya91
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v34-0006-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v30 4/6] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 74 ++++++++++++++++++++------------
 src/backend/commands/indexcmds.c | 39 +----------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 51 insertions(+), 64 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 532c11e9dd..b317f556df 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3607,7 +3607,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	Relation	iRel,
 				heapRelation;
 	Oid			heapId;
-	Oid			oldTablespaceOid;
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
@@ -3723,41 +3722,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
-	oldTablespaceOid = iRel->rd_rel->reltablespace;
 	if (set_tablespace &&
-		(tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+		set_rel_tablespace(indexId, tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
 		 * miss things.
 		 */
 		RelationDropStorage(iRel);
-
-		/* Update the pg_class row */
-		rd_rel->reltablespace = tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -4063,6 +4038,51 @@ reindex_relation(Oid relid, int flags, int options, Oid tablespaceOid)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index d13075e62c..af155938ac 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2832,41 +2832,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
@@ -2940,7 +2905,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel, char *tablespace)
 		 * relations.
 		 */
 		if (partkind == RELKIND_PARTITIONED_INDEX)
-			set_rel_tablespace(partoid, tablespaceOid);
+			(void) set_rel_tablespace(partoid, tablespaceOid);
 		else if (partkind == RELKIND_PARTITIONED_TABLE)
 		{
 			Relation rel = table_open(partoid, ShareLock);
@@ -2951,7 +2916,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel, char *tablespace)
 			foreach (lc, indexIds)
 			{
 				Oid indexid = lfirst_oid(lc);
-				set_rel_tablespace(indexid, tablespaceOid);
+				(void) set_rel_tablespace(indexid, tablespaceOid);
 			}
 		}
 
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 5de40f182f..ce32db62f9 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -140,6 +140,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, int options, Oid tablespaceOid);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--t0UkRYy7tHLRMCai
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v30-0005-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v31 3/5] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 74 ++++++++++++++++++++------------
 src/backend/commands/indexcmds.c | 35 ---------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 532c11e9dd..b317f556df 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3607,7 +3607,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	Relation	iRel,
 				heapRelation;
 	Oid			heapId;
-	Oid			oldTablespaceOid;
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
@@ -3723,41 +3722,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
-	oldTablespaceOid = iRel->rd_rel->reltablespace;
 	if (set_tablespace &&
-		(tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+		set_rel_tablespace(indexId, tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
 		 * miss things.
 		 */
 		RelationDropStorage(iRel);
-
-		/* Update the pg_class row */
-		rd_rel->reltablespace = tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -4063,6 +4038,51 @@ reindex_relation(Oid relid, int flags, int options, Oid tablespaceOid)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 59111b59d8..e16dd2e8a0 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2933,41 +2933,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 178ddce63c..14af380c5f 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -149,6 +149,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, int options, Oid tablespaceOid);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--2NVUe83uzBILE3UT
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v31-0005-Implement-vacuum-full-cluster-INDEX_TABLESPACE-t.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v32 3/5] Refactor and reuse set_rel_tablespace()
@ 2020-09-23 15:21 Alexey Kondratov <kondratov.aleksey@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Alexey Kondratov @ 2020-09-23 15:21 UTC (permalink / raw)

---
 src/backend/catalog/index.c      | 74 ++++++++++++++++++++------------
 src/backend/commands/indexcmds.c | 35 ---------------
 src/include/catalog/index.h      |  2 +
 3 files changed, 49 insertions(+), 62 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 532c11e9dd..b317f556df 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3607,7 +3607,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	Relation	iRel,
 				heapRelation;
 	Oid			heapId;
-	Oid			oldTablespaceOid;
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
@@ -3723,41 +3722,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 		tablespaceOid = InvalidOid;
 
 	/*
-	 * Set the new tablespace for the relation.  Do that only in the
-	 * case where the reindex caller wishes to enforce a new tablespace.
+	 * Set the new tablespace for the relation if requested.
 	 */
-	oldTablespaceOid = iRel->rd_rel->reltablespace;
 	if (set_tablespace &&
-		(tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid))))
+		set_rel_tablespace(indexId, tablespaceOid))
 	{
-		Relation		pg_class;
-		Form_pg_class	rd_rel;
-		HeapTuple		tuple;
-
-		/* First get a modifiable copy of the relation's pg_class row */
-		pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-		tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", indexId);
-		rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
 		/*
 		 * Mark the relation as ready to be dropped at transaction commit,
 		 * before making visible the new tablespace change so as this won't
 		 * miss things.
 		 */
 		RelationDropStorage(iRel);
-
-		/* Update the pg_class row */
-		rd_rel->reltablespace = tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
-		heap_freetuple(tuple);
-
-		table_close(pg_class, RowExclusiveLock);
-
 		RelationAssumeNewRelfilenode(iRel);
 
 		/* Make sure the reltablespace change is visible */
@@ -4063,6 +4038,51 @@ reindex_relation(Oid relid, int flags, int options, Oid tablespaceOid)
 	return result;
 }
 
+/*
+ * set_rel_tablespace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set.  Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+set_rel_tablespace(Oid reloid, Oid tablespaceOid)
+{
+	Relation		pg_class;
+	HeapTuple		tuple;
+	Form_pg_class	rd_rel;
+	bool			changed = false;
+	Oid			oldTablespaceOid;
+
+	/* Get a modifiable copy of the relation's pg_class row. */
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for relation %u", reloid);
+	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+	/* No work if no change in tablespace. */
+	oldTablespaceOid = rd_rel->reltablespace;
+	if (tablespaceOid != oldTablespaceOid ||
+		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
+	{
+		/* Update the pg_class row. */
+		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
+			InvalidOid : tablespaceOid;
+		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+		changed = true;
+	}
+
+	heap_freetuple(tuple);
+	table_close(pg_class, RowExclusiveLock);
+
+	return changed;
+}
 
 /* ----------------------------------------------------------------
  *		System index reindexing support
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 0c08dddbb7..a27f8f9d83 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2940,41 +2940,6 @@ reindex_error_callback(void *arg)
 				   errinfo->relnamespace, errinfo->relname);
 }
 
-/*
- * This is mostly duplicating ATExecSetTableSpaceNoStorage,
- * which should maybe be factored out to a library function.
- */
-static void
-set_rel_tablespace(Oid reloid, Oid tablespaceOid)
-{
-	Relation		pg_class;
-	HeapTuple		tuple;
-	Form_pg_class	rd_rel;
-	Oid			oldTablespaceOid;
-
-	/* Get a modifiable copy of the relation's pg_class row */
-	pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
-	tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
-	if (!HeapTupleIsValid(tuple))
-		elog(ERROR, "cache lookup failed for relation %u", reloid);
-	rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
-	/* No work if no change in tablespace. */
-	oldTablespaceOid = rd_rel->reltablespace;
-	if (tablespaceOid != oldTablespaceOid ||
-		(tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
-	{
-		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
-		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-	}
-
-	heap_freetuple(tuple);
-	table_close(pg_class, RowExclusiveLock);
-}
-
 /*
  * ReindexPartitions
  *
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 178ddce63c..14af380c5f 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -149,6 +149,8 @@ extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 						  char relpersistence, int options, Oid tablespaceOid);
 
+extern bool set_rel_tablespace(Oid reloid, Oid tablespaceOid);
+
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
 #define REINDEX_REL_SUPPRESS_INDEX_USE		0x02
-- 
2.17.0


--8jEihaNHb65WmIJG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v32-0004-Allow-CLUSTER-and-VACUUM-FULL-to-change-tablespa.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v44b 4/4] hack deadlock detector
@ 2026-03-25 19:35 Álvaro Herrera <alvherre@kurilemu.de>
  0 siblings, 0 replies; 8+ messages in thread

From: Álvaro Herrera @ 2026-03-25 19:35 UTC (permalink / raw)

---
 src/backend/commands/cluster.c                | 10 +++
 src/backend/storage/lmgr/deadlock.c           | 12 +++
 src/include/storage/proc.h                    |  1 +
 src/test/modules/injection_points/Makefile    |  1 +
 .../expected/repack_deadlock.out              | 63 ++++++++++++++
 src/test/modules/injection_points/meson.build |  1 +
 .../specs/repack_deadlock.spec                | 83 +++++++++++++++++++
 7 files changed, 171 insertions(+)
 create mode 100644 src/test/modules/injection_points/expected/repack_deadlock.out
 create mode 100644 src/test/modules/injection_points/specs/repack_deadlock.spec

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 2c3058ba10d..d5b1dfbff69 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -372,6 +372,16 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
 	/* Determine the lock mode to use. */
 	lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0);
 
+	/*
+	 * If in concurrent mode, set the PROC_IN_CONCURRENT_REPACK flag.  This
+	 * makes the deadlock checker cause anyone that would conflict with us
+	 * to error out.  It's important to set this flag ahead of actually locking
+	 * the relation; it won't of course affect anyone until we do have a lock
+	 * that others can conflict with.
+	 */
+	if ((params.options & CLUOPT_CONCURRENT) != 0)
+		MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
+
 	/*
 	 * If a single relation is specified, process it and we're done ... unless
 	 * the relation is a partitioned table, in which case we fall through.
diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index b8962d875b6..10266e384aa 100644
--- a/src/backend/storage/lmgr/deadlock.c
+++ b/src/backend/storage/lmgr/deadlock.c
@@ -620,6 +620,18 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
 						proc->statusFlags & PROC_IS_AUTOVACUUM)
 						blocking_autovacuum_proc = proc;
 
+					/*
+					 * If we note that we're blocked by some process running
+					 * REPACK (CONCURRENTLY), just fail.  That process is
+					 * going to upgrade its lock at some point, and it would
+					 * be inappropriate for any other process to cause that
+					 * to fail.
+					 */
+					if (checkProc == MyProc &&
+						proc->statusFlags & PROC_IN_CONCURRENT_REPACK)
+						/* not a serious error message proposal */
+						elog(ERROR, "oops, we're blocked by REPACK, time to die");
+
 					/* We're done looking at this proclock */
 					break;
 				}
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 1dad125706e..4e340ead455 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -69,6 +69,7 @@ struct XidCache
 #define		PROC_AFFECTS_ALL_HORIZONS	0x20	/* this proc's xmin must be
 												 * included in vacuum horizons
 												 * in all databases */
+#define		PROC_IN_CONCURRENT_REPACK	0x40	/* REPACK (CONCURRENTLY) */
 
 /* flags reset at EOXact */
 #define		PROC_VACUUM_STATE_MASK \
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2cd7d87c533..f7663859fe2 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -15,6 +15,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 ISOLATION = basic \
 	    inplace \
 	    repack \
+	    repack_deadlock \
 	    repack_toast \
 	    syscache-update-pruned \
 	    heap_lock_update
diff --git a/src/test/modules/injection_points/expected/repack_deadlock.out b/src/test/modules/injection_points/expected/repack_deadlock.out
new file mode 100644
index 00000000000..42505d10933
--- /dev/null
+++ b/src/test/modules/injection_points/expected/repack_deadlock.out
@@ -0,0 +1,63 @@
+Parsed test spec with 2 sessions
+
+starting permutation: wait_before_lock add_column wakeup_before_lock check1
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step wait_before_lock: 
+	REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey;
+ <waiting ...>
+step add_column: 
+	alter table repack_deadlock add column noise text;
+ <waiting ...>
+step add_column: <... completed>
+ERROR:  oops, we're blocked by REPACK, time to die
+step wakeup_before_lock: 
+	SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step wait_before_lock: <... completed>
+step check1: 
+	INSERT INTO relfilenodes(node)
+	SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock';
+
+	SELECT count(DISTINCT node) FROM relfilenodes;
+
+	SELECT i, j FROM repack_deadlock ORDER BY i, j;
+
+	INSERT INTO data_s1(i, j)
+	SELECT i, j FROM repack_deadlock;
+
+	SELECT count(*)
+	FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j)
+	WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+    1
+(1 row)
+
+i|j
+-+-
+1|1
+2|2
+3|3
+4|4
+(4 rows)
+
+count
+-----
+    4
+(1 row)
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index a414abb924b..1cd88d6db65 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -46,6 +46,7 @@ tests += {
       'basic',
       'inplace',
       'repack',
+      'repack_deadlock',
       'repack_toast',
       'syscache-update-pruned',
       'heap_lock_update',
diff --git a/src/test/modules/injection_points/specs/repack_deadlock.spec b/src/test/modules/injection_points/specs/repack_deadlock.spec
new file mode 100644
index 00000000000..9d23a6588c2
--- /dev/null
+++ b/src/test/modules/injection_points/specs/repack_deadlock.spec
@@ -0,0 +1,83 @@
+# Test REPACK with a concurrent transaction that would cause a deadlock
+setup
+{
+	CREATE EXTENSION injection_points;
+
+	CREATE TABLE repack_deadlock(i int PRIMARY KEY, j int);
+	INSERT INTO repack_deadlock(i, j) VALUES (1, 1), (2, 2), (3, 3), (4, 4);
+
+	CREATE TABLE relfilenodes(node oid);
+
+	CREATE TABLE data_s1(i int, j int);
+	CREATE TABLE data_s2(i int, j int);
+}
+
+teardown
+{
+	DROP TABLE repack_deadlock;
+	DROP EXTENSION injection_points;
+
+	DROP TABLE relfilenodes;
+	DROP TABLE data_s1;
+	DROP TABLE data_s2;
+}
+
+session s1
+setup
+{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
+}
+# Perform the initial load and wait for s2 to do some data changes.
+step wait_before_lock
+{
+	REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey;
+}
+# Check the table from the perspective of s1.
+#
+# Besides the contents, we also check that relfilenode has changed.
+
+# Have each session write the contents into a table and use FULL JOIN to check
+# if the outputs are identical.
+step check1
+{
+	INSERT INTO relfilenodes(node)
+	SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock';
+
+	SELECT count(DISTINCT node) FROM relfilenodes;
+
+	SELECT i, j FROM repack_deadlock ORDER BY i, j;
+
+	INSERT INTO data_s1(i, j)
+	SELECT i, j FROM repack_deadlock;
+
+	SELECT count(*)
+	FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j)
+	WHERE d1.i ISNULL OR d2.i ISNULL;
+}
+teardown
+{
+	SELECT injection_points_detach('repack-concurrently-before-lock');
+}
+
+session s2
+# Change the existing data. UPDATE changes both key and non-key columns. Also
+# update one row twice to test whether tuple version generated by this session
+# can be found.
+step add_column
+{
+	alter table repack_deadlock add column noise text;
+}
+
+step wakeup_before_lock
+{
+	SELECT injection_points_wakeup('repack-concurrently-before-lock');
+}
+
+# Test if data changes introduced while one session is performing REPACK
+# CONCURRENTLY find their way into the table.
+permutation
+	wait_before_lock
+	add_column
+	wakeup_before_lock
+	check1
-- 
2.47.3


--fot5plqmb33ey33c--





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

end of thread, other threads:[~2026-03-25 19:35 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 15:21 [PATCH 2/4] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2020-09-23 15:21 [PATCH v33 3/5] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2020-09-23 15:21 [PATCH v29 4/7] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2020-09-23 15:21 [PATCH v34 5/8] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2020-09-23 15:21 [PATCH v30 4/6] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2020-09-23 15:21 [PATCH v31 3/5] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2020-09-23 15:21 [PATCH v32 3/5] Refactor and reuse set_rel_tablespace() Alexey Kondratov <kondratov.aleksey@gmail.com>
2026-03-25 19:35 [PATCH v44b 4/4] hack deadlock detector Álvaro Herrera <alvherre@kurilemu.de>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox