From ed886f8202486dea8069b719d35a5d0db7f3277c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 6 Feb 2020 12:56:34 -0600
Subject: [PATCH v1 3/3] Make cluster a property of table in pg_index..

..rather than of indexes in pg_index.

The only issue with this is that it makes pg_class larger, and the new column
applies not only to tables, but to indices.
---
 doc/src/sgml/catalogs.sgml         |  14 +--
 src/backend/catalog/heap.c         |   1 +
 src/backend/catalog/index.c        |   6 --
 src/backend/commands/cluster.c     | 172 +++++++++++++++++--------------------
 src/backend/commands/tablecmds.c   |   5 +-
 src/backend/utils/cache/relcache.c |   1 -
 src/bin/psql/describe.c            |   4 +-
 src/include/catalog/pg_class.dat   |   2 +-
 src/include/catalog/pg_class.h     |   3 +
 src/include/catalog/pg_index.h     |   1 -
 10 files changed, 93 insertions(+), 116 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index a10b665..8efeaff 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1752,6 +1752,13 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
      </row>
 
      <row>
+      <entry><structfield>relclustered</structfield></entry>
+      <entry><type>oid</type></entry>
+      <entry></entry>
+      <entry>The OID of the index last clustered, or zero</entry>
+     </row>
+
+     <row>
       <entry><structfield>relpages</structfield></entry>
       <entry><type>int4</type></entry>
       <entry></entry>
@@ -3808,13 +3815,6 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
      </row>
 
      <row>
-      <entry><structfield>indisclustered</structfield></entry>
-      <entry><type>bool</type></entry>
-      <entry></entry>
-      <entry>If true, the table was last clustered on this index</entry>
-     </row>
-
-     <row>
       <entry><structfield>indisvalid</structfield></entry>
       <entry><type>bool</type></entry>
       <entry></entry>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 046b3d3..5769d15 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -901,6 +901,7 @@ InsertPgClassTuple(Relation pg_class_desc,
 	values[Anum_pg_class_relam - 1] = ObjectIdGetDatum(rd_rel->relam);
 	values[Anum_pg_class_relfilenode - 1] = ObjectIdGetDatum(rd_rel->relfilenode);
 	values[Anum_pg_class_reltablespace - 1] = ObjectIdGetDatum(rd_rel->reltablespace);
+	values[Anum_pg_class_relclustered - 1] = ObjectIdGetDatum(rd_rel->relclustered);
 	values[Anum_pg_class_relpages - 1] = Int32GetDatum(rd_rel->relpages);
 	values[Anum_pg_class_reltuples - 1] = Float4GetDatum(rd_rel->reltuples);
 	values[Anum_pg_class_relallvisible - 1] = Int32GetDatum(rd_rel->relallvisible);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 8880586..34ba896 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -607,7 +607,6 @@ UpdateIndexRelation(Oid indexoid,
 	values[Anum_pg_index_indisprimary - 1] = BoolGetDatum(primary);
 	values[Anum_pg_index_indisexclusion - 1] = BoolGetDatum(isexclusion);
 	values[Anum_pg_index_indimmediate - 1] = BoolGetDatum(immediate);
-	values[Anum_pg_index_indisclustered - 1] = BoolGetDatum(false);
 	values[Anum_pg_index_indisvalid - 1] = BoolGetDatum(isvalid);
 	values[Anum_pg_index_indcheckxmin - 1] = BoolGetDatum(false);
 	values[Anum_pg_index_indisready - 1] = BoolGetDatum(isready);
@@ -1530,7 +1529,6 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
 	/* Mark old index as valid and new as invalid as index_set_state_flags */
 	newIndexForm->indisvalid = true;
 	oldIndexForm->indisvalid = false;
-	oldIndexForm->indisclustered = false;
 
 	CatalogTupleUpdate(pg_index, &oldIndexTuple->t_self, oldIndexTuple);
 	CatalogTupleUpdate(pg_index, &newIndexTuple->t_self, newIndexTuple);
@@ -3351,12 +3349,8 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action)
 			 * indisvalid is false.  (We don't assert that either is initially
 			 * true, though, since we want to be able to retry a DROP INDEX
 			 * CONCURRENTLY that failed partway through.)
-			 *
-			 * Note: the CLUSTER logic assumes that indisclustered cannot be
-			 * set on any invalid index, so clear that flag too.
 			 */
 			indexForm->indisvalid = false;
-			indexForm->indisclustered = false;
 			break;
 		case INDEX_DROP_SET_DEAD:
 
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 3adcbeb..187e117 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -94,8 +94,8 @@ static List *get_tables_to_cluster(MemoryContext cluster_context);
  * The single-relation case does not have any such overhead.
  *
  * We also allow a relation to be specified without index.  In that case,
- * the indisclustered bit will be looked up, and an ERROR will be thrown
- * if there is no index with the bit set.
+ * the clustered index will be looked up, and an ERROR will be thrown
+ * if no index is clustered.
  *---------------------------------------------------------------------------
  */
 void
@@ -134,28 +134,17 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
 
 		if (stmt->indexname == NULL)
 		{
-			ListCell   *index;
-
-			/* We need to find the index that has indisclustered set. */
-			foreach(index, RelationGetIndexList(rel))
-			{
-				HeapTuple	idxtuple;
-				Form_pg_index indexForm;
-
-				indexOid = lfirst_oid(index);
-				idxtuple = SearchSysCache1(INDEXRELID,
-										   ObjectIdGetDatum(indexOid));
-				if (!HeapTupleIsValid(idxtuple))
-					elog(ERROR, "cache lookup failed for index %u", indexOid);
-				indexForm = (Form_pg_index) GETSTRUCT(idxtuple);
-				if (indexForm->indisclustered)
-				{
-					ReleaseSysCache(idxtuple);
-					break;
-				}
-				ReleaseSysCache(idxtuple);
-				indexOid = InvalidOid;
-			}
+			/* Lookup the clustered index */
+			Form_pg_class classForm;
+			HeapTuple	tuple;
+
+			tuple = SearchSysCache1(RELOID,
+									   ObjectIdGetDatum(tableOid));
+			if (!HeapTupleIsValid(tuple))
+				elog(ERROR, "cache lookup failed for relation %u", tableOid);
+			classForm = (Form_pg_class) GETSTRUCT(tuple);
+			indexOid = classForm->relclustered;
+			ReleaseSysCache(tuple);
 
 			if (!OidIsValid(indexOid))
 				ereport(ERROR,
@@ -188,7 +177,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
 	{
 		/*
 		 * This is the "multi relation" case. We need to cluster all tables
-		 * that have some index with indisclustered set.
+		 * that have cluster_index set.
 		 */
 		MemoryContext cluster_context;
 		List	   *rvs;
@@ -304,8 +293,8 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
 	 */
 	if (recheck)
 	{
-		HeapTuple	tuple;
-		Form_pg_index indexForm;
+		HeapTuple		tuple;
+		Form_pg_class	classForm;
 
 		/* Check that the user still owns the relation */
 		if (!pg_class_ownercheck(tableOid, GetUserId()))
@@ -343,17 +332,17 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
 			}
 
 			/*
-			 * Check that the index is still the one with indisclustered set.
+			 * Check that the index is still the one clustered.
 			 */
-			tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
+			tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(indexOid));
 			if (!HeapTupleIsValid(tuple))	/* probably can't happen */
 			{
 				relation_close(OldHeap, AccessExclusiveLock);
 				pgstat_progress_end_command();
 				return;
 			}
-			indexForm = (Form_pg_index) GETSTRUCT(tuple);
-			if (!indexForm->indisclustered)
+			classForm = (Form_pg_class) GETSTRUCT(tuple);
+			if (classForm->relclustered != indexOid)
 			{
 				ReleaseSysCache(tuple);
 				relation_close(OldHeap, AccessExclusiveLock);
@@ -366,8 +355,8 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
 
 	/*
 	 * We allow VACUUM FULL, but not CLUSTER, on shared catalogs.  CLUSTER
-	 * would work in most respects, but the index would only get marked as
-	 * indisclustered in the current database, leading to unexpected behavior
+	 * would work in most respects, but the table would only be marked as
+	 * clustered in the current database, leading to unexpected behavior
 	 * if CLUSTER were later invoked in another database.
 	 */
 	if (OidIsValid(indexOid) && OldHeap->rd_rel->relisshared)
@@ -498,15 +487,14 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
 /*
  * mark_index_clustered: mark the specified index as the one clustered on
  *
- * With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
+ * With indexOid == InvalidOid, will mark as not-clustered.
  */
 void
 mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
 {
-	HeapTuple	indexTuple;
-	Form_pg_index indexForm;
-	Relation	pg_index;
-	ListCell   *index;
+	HeapTuple	tuple;
+	Form_pg_class classForm;
+	Relation	pg_class;
 
 	/* Disallow applying to a partitioned table */
 	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
@@ -516,63 +504,45 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
 
 	/*
 	 * If the index is already marked clustered, no need to do anything.
+	 * XXX: just remove this?
 	 */
 	if (OidIsValid(indexOid))
 	{
-		indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
-		if (!HeapTupleIsValid(indexTuple))
-			elog(ERROR, "cache lookup failed for index %u", indexOid);
-		indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+		tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(rel->rd_id));
+		if (!HeapTupleIsValid(tuple))
+			elog(ERROR, "cache lookup failed for relation %u", rel->rd_id);
+		classForm = (Form_pg_class) GETSTRUCT(tuple);
 
-		if (indexForm->indisclustered)
+		if (classForm->relclustered == indexOid)
 		{
-			ReleaseSysCache(indexTuple);
+			ReleaseSysCache(tuple);
 			return;
 		}
 
-		ReleaseSysCache(indexTuple);
+		ReleaseSysCache(tuple);
 	}
 
 	/*
-	 * Check each index of the relation and set/clear the bit as needed.
+	 * Need to set the clustered index.
 	 */
-	pg_index = table_open(IndexRelationId, RowExclusiveLock);
-
-	foreach(index, RelationGetIndexList(rel))
-	{
-		Oid			thisIndexOid = lfirst_oid(index);
+	pg_class = table_open(RelationRelationId, RowExclusiveLock);
 
-		indexTuple = SearchSysCacheCopy1(INDEXRELID,
-										 ObjectIdGetDatum(thisIndexOid));
-		if (!HeapTupleIsValid(indexTuple))
-			elog(ERROR, "cache lookup failed for index %u", thisIndexOid);
-		indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+	tuple = SearchSysCacheCopy1(RELOID,
+									 ObjectIdGetDatum(rel->rd_id));
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "cache lookup failed for index %u", indexOid);
+	classForm = (Form_pg_class) GETSTRUCT(tuple);
 
-		/*
-		 * Unset the bit if set.  We know it's wrong because we checked this
-		 * earlier.
-		 */
-		if (indexForm->indisclustered)
-		{
-			indexForm->indisclustered = false;
-			CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
-		}
-		else if (thisIndexOid == indexOid)
-		{
-			/* this was checked earlier, but let's be real sure */
-			if (!indexForm->indisvalid)
-				elog(ERROR, "cannot cluster on invalid index %u", indexOid);
-			indexForm->indisclustered = true;
-			CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
-		}
+	classForm->relclustered = indexOid;
+	CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
 
-		InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
-									 InvalidOid, is_internal);
+	InvokeObjectPostAlterHookArg(RelationRelationId, rel->rd_id, 0,
+								 InvalidOid, is_internal);
 
-		heap_freetuple(indexTuple);
-	}
+	heap_freetuple(tuple);
+	// ReleaseSysCache(tuple);
 
-	table_close(pg_index, RowExclusiveLock);
+	table_close(pg_class, RowExclusiveLock);
 }
 
 /*
@@ -1521,41 +1491,53 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 
 /*
  * Get a list of tables that the current user owns and
- * have indisclustered set.  Return the list in a List * of RelToCluster
+ * have a relclustered.  Return the list in a List * of RelToCluster
  * with the tableOid and the indexOid on which the table is already
  * clustered.
  */
 static List *
 get_tables_to_cluster(MemoryContext cluster_context)
 {
-	Relation	indRelation;
+	Relation	clsRelation;
 	TableScanDesc scan;
 	ScanKeyData entry;
-	HeapTuple	indexTuple;
-	Form_pg_index index;
+	HeapTuple	tuple;
 	MemoryContext old_context;
 	RelToCluster *rvtc;
 	List	   *rvs = NIL;
 
 	/*
-	 * Get all indexes that have indisclustered set and are owned by
-	 * appropriate user. Shared relations cannot ever have indisclustered
+	 * Get all tables with clustered indexes and whcih are owned by
+	 * appropriate user.  Shared relations cannot ever have indisclustered
 	 * set, because CLUSTER will refuse to set it when called with one as
 	 * an argument.
 	 */
-	indRelation = table_open(IndexRelationId, AccessShareLock);
+	clsRelation = table_open(RelationRelationId, AccessShareLock);
 	ScanKeyInit(&entry,
-				Anum_pg_index_indisclustered,
-				BTEqualStrategyNumber, F_BOOLEQ,
-				BoolGetDatum(true));
-	scan = table_beginscan_catalog(indRelation, 1, &entry);
-	while ((indexTuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+				Anum_pg_class_relclustered,
+				BTGreaterStrategyNumber, F_OIDEQ,
+				(Datum) 0);
+
+	scan = table_beginscan_catalog(clsRelation, 1, &entry);
+	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
 	{
-		index = (Form_pg_index) GETSTRUCT(indexTuple);
+		Form_pg_class classForm;
+		HeapTuple	indtuple;
+		classForm = (Form_pg_class) GETSTRUCT(tuple);
+
+		if (!pg_class_ownercheck(classForm->oid, GetUserId()))
+			continue;
 
-		if (!pg_class_ownercheck(index->indrelid, GetUserId()))
+		if (!OidIsValid(classForm->relclustered)) /* Shouldn't happen */
 			continue;
 
+		indtuple = SearchSysCache1(RELOID, ObjectIdGetDatum(classForm->relclustered)); // XXX: check that it's an index, and on that relation?
+		if (!HeapTupleIsValid(indtuple)) {
+			ReleaseSysCache(indtuple);
+			continue;
+		}
+		ReleaseSysCache(indtuple);
+
 		/*
 		 * We have to build the list in a different memory context so it will
 		 * survive the cross-transaction processing
@@ -1563,15 +1545,15 @@ get_tables_to_cluster(MemoryContext cluster_context)
 		old_context = MemoryContextSwitchTo(cluster_context);
 
 		rvtc = (RelToCluster *) palloc(sizeof(RelToCluster));
-		rvtc->tableOid = index->indrelid;
-		rvtc->indexOid = index->indexrelid;
+		rvtc->tableOid = classForm->oid;
+		rvtc->indexOid = classForm->relclustered;
 		rvs = lappend(rvs, rvtc);
 
 		MemoryContextSwitchTo(old_context);
 	}
 	table_endscan(scan);
 
-	relation_close(indRelation, AccessShareLock);
+	relation_close(clsRelation, AccessShareLock);
 
 	return rvs;
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f599393..ca2c1f1 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12547,7 +12547,7 @@ change_owner_recurse_to_sequences(Oid relationOid, Oid newOwnerId, LOCKMODE lock
 /*
  * ALTER TABLE CLUSTER ON
  *
- * The only thing we have to do is to change the indisclustered bits.
+ * The only thing we have to do is to change relclustered.
  *
  * Return the address of the new clustering index.
  */
@@ -12580,8 +12580,7 @@ ATExecClusterOn(Relation rel, const char *indexName, LOCKMODE lockmode)
 /*
  * ALTER TABLE SET WITHOUT CLUSTER
  *
- * We have to find any indexes on the table that have indisclustered bit
- * set and turn it off.
+ * We have to unset relclustered.
  */
 static void
 ATExecDropCluster(Relation rel, LOCKMODE lockmode)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index df025a5..9dae783 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2206,7 +2206,6 @@ RelationReloadIndexInfo(Relation relation)
 		relation->rd_index->indisprimary = index->indisprimary;
 		relation->rd_index->indisexclusion = index->indisexclusion;
 		relation->rd_index->indimmediate = index->indimmediate;
-		relation->rd_index->indisclustered = index->indisclustered;
 		relation->rd_index->indisvalid = index->indisvalid;
 		relation->rd_index->indcheckxmin = index->indcheckxmin;
 		relation->rd_index->indisready = index->indisready;
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index f3c7eb9..71ec575 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2205,7 +2205,7 @@ describeOneTableDetails(const char *schemaname,
 		PGresult   *result;
 
 		printfPQExpBuffer(&buf,
-						  "SELECT i.indisunique, i.indisprimary, i.indisclustered, ");
+						  "SELECT i.indisunique, i.indisprimary, c.relclustered=i.indexrelid,");
 		if (pset.sversion >= 80200)
 			appendPQExpBufferStr(&buf, "i.indisvalid,\n");
 		else
@@ -2319,7 +2319,7 @@ describeOneTableDetails(const char *schemaname,
 		if (tableinfo.hasindex)
 		{
 			printfPQExpBuffer(&buf,
-							  "SELECT c2.relname, i.indisprimary, i.indisunique, i.indisclustered, ");
+							  "SELECT c2.relname, i.indisprimary, i.indisunique, c.relclustered=i.indexrelid,");
 			if (pset.sversion >= 80200)
 				appendPQExpBufferStr(&buf, "i.indisvalid, ");
 			else
diff --git a/src/include/catalog/pg_class.dat b/src/include/catalog/pg_class.dat
index f70d5ba..0ea1b10 100644
--- a/src/include/catalog/pg_class.dat
+++ b/src/include/catalog/pg_class.dat
@@ -54,7 +54,7 @@
   relname => 'pg_class', reltype => 'pg_class', relam => 'heap',
   relfilenode => '0', relpages => '0', reltuples => '0', relallvisible => '0',
   reltoastrelid => '0', relhasindex => 'f', relisshared => 'f',
-  relpersistence => 'p', relkind => 'r', relnatts => '33', relchecks => '0',
+  relpersistence => 'p', relkind => 'r', relnatts => '34', relchecks => '0',
   relhasrules => 'f', relhastriggers => 'f', relhassubclass => 'f',
   relrowsecurity => 'f', relforcerowsecurity => 'f', relispopulated => 't',
   relreplident => 'n', relispartition => 'f', relfrozenxid => '3',
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index a12fc1f..228e714 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -119,6 +119,9 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
 	/* heap for rewrite during DDL, link to original rel */
 	Oid			relrewrite BKI_DEFAULT(0);
 
+	/* identifier of index last clustered by: XXX PG_INDEX XXX */
+	Oid			relclustered BKI_DEFAULT(0);
+
 	/* all Xids < this are frozen in this rel */
 	TransactionId relfrozenxid;
 
diff --git a/src/include/catalog/pg_index.h b/src/include/catalog/pg_index.h
index d3d7ea7..e39e23d 100644
--- a/src/include/catalog/pg_index.h
+++ b/src/include/catalog/pg_index.h
@@ -36,7 +36,6 @@ CATALOG(pg_index,2610,IndexRelationId) BKI_SCHEMA_MACRO
 	bool		indisprimary;	/* is this index for primary key? */
 	bool		indisexclusion; /* is this index for exclusion constraint? */
 	bool		indimmediate;	/* is uniqueness enforced immediately? */
-	bool		indisclustered; /* is this the index last clustered by? */
 	bool		indisvalid;		/* is this index valid for use by queries? */
 	bool		indcheckxmin;	/* must we wait for xmin to be old? */
 	bool		indisready;		/* is this index ready for inserts? */
-- 
2.7.4

