From 02a8d0c45253eb54e57b1974c8627e5be3e1d852 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 20 Dec 2024 23:22:37 +1300 Subject: [PATCH] Remove pg_attribute.attcacheoff column The column is no longer needed as the offset is now cached in the CompactAttribute struct per commit 5983a4cff. Author: David Rowley Reviewed-by: Andres Freund, Victor Yegorov Discussion: https://postgr.es/m/CAApHDvrBztXP3yx=NKNmo3xwFAFhEdyPnvrDg3=M0RhDs+4vYw@mail.gmail.com --- 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 | 17 ----------------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_attribute.h | 9 --------- src/test/regress/expected/type_sanity.out | 3 +-- src/test/regress/sql/type_sanity.sql | 3 +-- 10 files changed, 9 insertions(+), 71 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index bf3cee08a9..cc6cf9bef0 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1186,17 +1186,6 @@ - - - attcacheoff int4 - - - 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 - - - atttypmod int4 diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index 4d89acbe5e..1a8d6481a2 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -388,17 +388,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; @@ -528,9 +518,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; @@ -771,7 +760,6 @@ TupleDescInitEntry(TupleDesc desc, else if (attributeName != NameStr(att->attname)) namestrcpy(&(att->attname), attributeName); - att->attcacheoff = -1; att->atttypmod = typmod; att->attnum = attributeNumber; @@ -835,7 +823,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 d35ccab487..e0cb70ee9d 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -580,7 +580,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 d7b88b61dc..7015967e88 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 6200a0da50..6976249e9e 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 5658e4accb..1ce7eb9da8 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -661,19 +661,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. - */ -#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 @@ -1964,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); } @@ -4401,8 +4386,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/catversion.h b/src/include/catalog/catversion.h index 7931e78c02..76d692772c 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202412191 +#define CATALOG_VERSION_NO 202412201 #endif 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.39.5