From 66b9bc418f7a79fbe98c7dd644da1ffbe1f65a62 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 6 Aug 2024 03:03:12 +1200 Subject: [PATCH v2 4/4] Remove pg_attribute.attcacheoff column This is no longer needed as the offset is now cached in the CompactAttribute struct. --- 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 b654fae1b2..6c6abd53ca 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 4252904fc8..e2aaa38bd4 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -368,17 +368,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; @@ -508,9 +498,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; @@ -751,7 +740,6 @@ TupleDescInitEntry(TupleDesc desc, else if (attributeName != NameStr(att->attname)) namestrcpy(&(att->attname), attributeName); - att->attcacheoff = -1; att->atttypmod = typmod; att->attnum = attributeNumber; @@ -816,7 +804,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 7637581a18..e6b4ff6f10 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -547,7 +547,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 01b43cc6a8..9a9f4c1562 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 6ffc536317..d0dc7d049a 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 71c4626b7a..d6200dc6db 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -662,20 +662,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 @@ -1967,8 +1953,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(TupleDescCompactAttr(relation->rd_att, i), TupleDescAttr(relation->rd_att, i)); @@ -4439,8 +4423,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(TupleDescCompactAttr(result, i), TupleDescAttr(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