From 349110ca0985aabc521c65ddd67e4e6746fde1d2 Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Tue, 6 Aug 2024 03:03:12 +1200
Subject: [PATCH v4 4/6] Remove pg_attribute.attcacheoff column

This is no longer needed as the offset is now cached in the
CompactAttribute struct.

catversion bump
---
 doc/src/sgml/catalogs.sgml                | 11 -----------
 src/backend/access/common/tupdesc.c       | 17 ++---------------
 src/backend/bootstrap/bootstrap.c         |  1 -
 src/backend/catalog/heap.c                | 16 ++++------------
 src/backend/catalog/index.c               |  1 -
 src/backend/utils/cache/relcache.c        | 18 ------------------
 src/include/catalog/pg_attribute.h        |  9 ---------
 src/test/regress/expected/type_sanity.out |  3 +--
 src/test/regress/sql/type_sanity.sql      |  3 +--
 9 files changed, 8 insertions(+), 71 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 964c819a02..0436c495eb 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1186,17 +1186,6 @@
       </para></entry>
      </row>
 
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>attcacheoff</structfield> <type>int4</type>
-      </para>
-      <para>
-       Always -1 in storage, but when loaded into a row descriptor
-       in memory this might be updated to cache the offset of the attribute
-       within the row
-      </para></entry>
-     </row>
-
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>atttypmod</structfield> <type>int4</type>
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 78c26e769f..363226c967 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -361,17 +361,7 @@ TupleDescCopyEntry(TupleDesc dst, AttrNumber dstAttno,
 
 	memcpy(dstAtt, srcAtt, ATTRIBUTE_FIXED_PART_SIZE);
 
-	/*
-	 * Aside from updating the attno, we'd better reset attcacheoff.
-	 *
-	 * XXX Actually, to be entirely safe we'd need to reset the attcacheoff of
-	 * all following columns in dst as well.  Current usage scenarios don't
-	 * require that though, because all following columns will get initialized
-	 * by other uses of this function or TupleDescInitEntry.  So we cheat a
-	 * bit to avoid a useless O(N^2) penalty.
-	 */
 	dstAtt->attnum = dstAttno;
-	dstAtt->attcacheoff = -1;
 
 	/* since we're not copying constraints or defaults, clear these */
 	dstAtt->attnotnull = false;
@@ -501,9 +491,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
 		 * them (since atttypid will be zero for all dropped columns) and in
 		 * general it seems safer to check them always.
 		 *
-		 * attcacheoff must NOT be checked since it's possibly not set in both
-		 * copies.  We also intentionally ignore atthasmissing, since that's
-		 * not very relevant in tupdescs, which lack the attmissingval field.
+		 * We intentionally ignore atthasmissing, since that's not very
+		 * relevant in tupdescs, which lack the attmissingval field.
 		 */
 		if (strcmp(NameStr(attr1->attname), NameStr(attr2->attname)) != 0)
 			return false;
@@ -744,7 +733,6 @@ TupleDescInitEntry(TupleDesc desc,
 	else if (attributeName != NameStr(att->attname))
 		namestrcpy(&(att->attname), attributeName);
 
-	att->attcacheoff = -1;
 	att->atttypmod = typmod;
 
 	att->attnum = attributeNumber;
@@ -808,7 +796,6 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
 	Assert(attributeName != NULL);
 	namestrcpy(&(att->attname), attributeName);
 
-	att->attcacheoff = -1;
 	att->atttypmod = typmod;
 
 	att->attnum = attributeNumber;
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index ed59dfce89..77dcca4b7a 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -549,7 +549,6 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
 	if (OidIsValid(attrtypes[attnum]->attcollation))
 		attrtypes[attnum]->attcollation = C_COLLATION_OID;
 
-	attrtypes[attnum]->attcacheoff = -1;
 	attrtypes[attnum]->atttypmod = -1;
 	attrtypes[attnum]->attislocal = true;
 
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 78e59384d1..95deedbee6 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -144,7 +144,6 @@ static const FormData_pg_attribute a1 = {
 	.atttypid = TIDOID,
 	.attlen = sizeof(ItemPointerData),
 	.attnum = SelfItemPointerAttributeNumber,
-	.attcacheoff = -1,
 	.atttypmod = -1,
 	.attbyval = false,
 	.attalign = TYPALIGN_SHORT,
@@ -158,7 +157,6 @@ static const FormData_pg_attribute a2 = {
 	.atttypid = XIDOID,
 	.attlen = sizeof(TransactionId),
 	.attnum = MinTransactionIdAttributeNumber,
-	.attcacheoff = -1,
 	.atttypmod = -1,
 	.attbyval = true,
 	.attalign = TYPALIGN_INT,
@@ -172,7 +170,6 @@ static const FormData_pg_attribute a3 = {
 	.atttypid = CIDOID,
 	.attlen = sizeof(CommandId),
 	.attnum = MinCommandIdAttributeNumber,
-	.attcacheoff = -1,
 	.atttypmod = -1,
 	.attbyval = true,
 	.attalign = TYPALIGN_INT,
@@ -186,7 +183,6 @@ static const FormData_pg_attribute a4 = {
 	.atttypid = XIDOID,
 	.attlen = sizeof(TransactionId),
 	.attnum = MaxTransactionIdAttributeNumber,
-	.attcacheoff = -1,
 	.atttypmod = -1,
 	.attbyval = true,
 	.attalign = TYPALIGN_INT,
@@ -200,7 +196,6 @@ static const FormData_pg_attribute a5 = {
 	.atttypid = CIDOID,
 	.attlen = sizeof(CommandId),
 	.attnum = MaxCommandIdAttributeNumber,
-	.attcacheoff = -1,
 	.atttypmod = -1,
 	.attbyval = true,
 	.attalign = TYPALIGN_INT,
@@ -220,7 +215,6 @@ static const FormData_pg_attribute a6 = {
 	.atttypid = OIDOID,
 	.attlen = sizeof(Oid),
 	.attnum = TableOidAttributeNumber,
-	.attcacheoff = -1,
 	.atttypmod = -1,
 	.attbyval = true,
 	.attalign = TYPALIGN_INT,
@@ -684,11 +678,10 @@ CheckAttributeType(const char *attname,
  *		Construct and insert a set of tuples in pg_attribute.
  *
  * Caller has already opened and locked pg_attribute.  tupdesc contains the
- * attributes to insert.  attcacheoff is always initialized to -1.
- * tupdesc_extra supplies the values for certain variable-length/nullable
- * pg_attribute fields and must contain the same number of elements as tupdesc
- * or be NULL.  The other variable-length fields of pg_attribute are always
- * initialized to null values.
+ * attributes to insert.  tupdesc_extra supplies the values for certain
+ * variable-length/nullable pg_attribute fields and must contain the same
+ * number of elements as tupdesc or be NULL.  The other variable-length fields
+ * of pg_attribute are always initialized to null values.
  *
  * indstate is the index state for CatalogTupleInsertWithInfo.  It can be
  * passed as NULL, in which case we'll fetch the necessary info.  (Don't do
@@ -740,7 +733,6 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
 		slot[slotCount]->tts_values[Anum_pg_attribute_atttypid - 1] = ObjectIdGetDatum(attrs->atttypid);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attlen - 1] = Int16GetDatum(attrs->attlen);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attnum - 1] = Int16GetDatum(attrs->attnum);
-		slot[slotCount]->tts_values[Anum_pg_attribute_attcacheoff - 1] = Int32GetDatum(-1);
 		slot[slotCount]->tts_values[Anum_pg_attribute_atttypmod - 1] = Int32GetDatum(attrs->atttypmod);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attndims - 1] = Int16GetDatum(attrs->attndims);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attbyval - 1] = BoolGetDatum(attrs->attbyval);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 80bb1ee507..cd9e2d54a3 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -320,7 +320,6 @@ ConstructTupleDescriptor(Relation heapRelation,
 
 		MemSet(to, 0, ATTRIBUTE_FIXED_PART_SIZE);
 		to->attnum = i + 1;
-		to->attcacheoff = -1;
 		to->attislocal = true;
 		to->attcollation = (i < numkeyatts) ? collationIds[i] : InvalidOid;
 
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index cd7e705556..8c48997f6b 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -660,20 +660,6 @@ RelationBuildTupleDesc(Relation relation)
 		elog(ERROR, "pg_attribute catalog is missing %d attribute(s) for relation OID %u",
 			 need, RelationGetRelid(relation));
 
-	/*
-	 * The attcacheoff values we read from pg_attribute should all be -1
-	 * ("unknown").  Verify this if assert checking is on.  They will be
-	 * computed when and if needed during tuple access.
-	 */
-#ifdef USE_ASSERT_CHECKING
-	{
-		int			i;
-
-		for (i = 0; i < RelationGetNumberOfAttributes(relation); i++)
-			Assert(TupleDescAttr(relation->rd_att, i)->attcacheoff == -1);
-	}
-#endif
-
 	/*
 	 * We can easily set the attcacheoff value for the first attribute: it
 	 * must be zero.  This eliminates the need for special cases for attnum=1
@@ -1965,8 +1951,6 @@ formrdesc(const char *relationName, Oid relationReltype,
 			   &attrs[i],
 			   ATTRIBUTE_FIXED_PART_SIZE);
 		has_not_null |= attrs[i].attnotnull;
-		/* make sure attcacheoff is valid */
-		TupleDescAttr(relation->rd_att, i)->attcacheoff = -1;
 
 		populate_compact_attribute(relation->rd_att, i);
 	}
@@ -4440,8 +4424,6 @@ BuildHardcodedDescriptor(int natts, const FormData_pg_attribute *attrs)
 	for (i = 0; i < natts; i++)
 	{
 		memcpy(TupleDescAttr(result, i), &attrs[i], ATTRIBUTE_FIXED_PART_SIZE);
-		/* make sure attcacheoff is valid */
-		TupleDescAttr(result, i)->attcacheoff = -1;
 
 		populate_compact_attribute(result, i);
 	}
diff --git a/src/include/catalog/pg_attribute.h b/src/include/catalog/pg_attribute.h
index 1c62b8bfcb..30d1e8cfcc 100644
--- a/src/include/catalog/pg_attribute.h
+++ b/src/include/catalog/pg_attribute.h
@@ -73,15 +73,6 @@ CATALOG(pg_attribute,1249,AttributeRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(75,
 	 */
 	int16		attnum;
 
-	/*
-	 * fastgetattr() uses attcacheoff to cache byte offsets of attributes in
-	 * heap tuples.  The value actually stored in pg_attribute (-1) indicates
-	 * no cached value.  But when we copy these tuples into a tuple
-	 * descriptor, we may then update attcacheoff in the copies. This speeds
-	 * up the attribute walking process.
-	 */
-	int32		attcacheoff BKI_DEFAULT(-1);
-
 	/*
 	 * atttypmod records type-specific data supplied at table creation time
 	 * (for example, the max length of a varchar field).  It is passed to
diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out
index 88d8f6c32d..8eff3d10d2 100644
--- a/src/test/regress/expected/type_sanity.out
+++ b/src/test/regress/expected/type_sanity.out
@@ -550,8 +550,7 @@ WHERE pc.relkind IN ('r', 't', 'm') and
 SELECT a1.attrelid, a1.attname
 FROM pg_attribute as a1
 WHERE a1.attrelid = 0 OR a1.atttypid = 0 OR a1.attnum = 0 OR
-    a1.attcacheoff != -1 OR a1.attinhcount < 0 OR
-    (a1.attinhcount = 0 AND NOT a1.attislocal);
+    a1.attinhcount < 0 OR (a1.attinhcount = 0 AND NOT a1.attislocal);
  attrelid | attname 
 ----------+---------
 (0 rows)
diff --git a/src/test/regress/sql/type_sanity.sql b/src/test/regress/sql/type_sanity.sql
index e88d6cbe49..303f90955d 100644
--- a/src/test/regress/sql/type_sanity.sql
+++ b/src/test/regress/sql/type_sanity.sql
@@ -397,8 +397,7 @@ WHERE pc.relkind IN ('r', 't', 'm') and
 SELECT a1.attrelid, a1.attname
 FROM pg_attribute as a1
 WHERE a1.attrelid = 0 OR a1.atttypid = 0 OR a1.attnum = 0 OR
-    a1.attcacheoff != -1 OR a1.attinhcount < 0 OR
-    (a1.attinhcount = 0 AND NOT a1.attislocal);
+    a1.attinhcount < 0 OR (a1.attinhcount = 0 AND NOT a1.attislocal);
 
 -- Cross-check attnum against parent relation
 
-- 
2.34.1

