From 2470ca435c452fe4def9dcc4a831b5101691d541 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Wed, 22 Oct 2025 11:36:26 +1300 Subject: Use CompactAttribute more often, when possible 5983a4cff added CompactAttribute for storing commonly used fields from FormData_pg_attribute. 5983a4cff didn't go to the trouble of adjusting every location where we can use CompactAttribute rather than FormData_pg_attribute, so here we change the remaining ones. There are some locations where I've left the code using FormData_pg_attribute. These are mostly in the ALTER TABLE code. Using CompactAttribute here seems more risky as often the TupleDesc is being changed and those changes may not have been flushed to the CompactAttribute yet. I've also left record_recv(), record_send(), record_cmp(), record_eq() and record_image_eq() alone as it's not clear to me that accessing the CompactAttribute is a win here due to the FormData_pg_attribute still having to be accessed for most cases. Switching the relevant parts to use CompactAttribute would result in having to access both for common cases. Careful benchmarking may reveal that something can be done to make this better, but in absence of that, the safer option is to leave these alone. In ReorderBufferToastReplace(), there was a check to skip attnums < 0 while looping over the TupleDesc. Doing this is redundant since TupleDescs don't store < 0 attnums. Removing that code allows us to move to using CompactAttribute. The change in validateDomainCheckConstraint() just moves fetching the FormData_pg_attribute into the ERROR path, which is cold due to calling errstart_cold() and results in code being moved out of the common path. Author: David Rowley Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAApHDvrMy90o1Lgkt31F82tcSuwRFHq3vyGewSRN=-QuSEEvyQ@mail.gmail.com --- contrib/dblink/dblink.c | 2 +- contrib/file_fdw/file_fdw.c | 4 +--- contrib/postgres_fdw/deparse.c | 10 ++++------ src/backend/access/gin/gininsert.c | 2 +- src/backend/commands/typecmds.c | 3 ++- src/backend/optimizer/util/plancat.c | 2 +- src/backend/parser/parse_relation.c | 8 ++++---- src/backend/replication/logical/reorderbuffer.c | 6 +----- src/backend/replication/logical/worker.c | 5 +++-- src/backend/replication/pgoutput/pgoutput.c | 2 +- src/backend/utils/adt/expandedrecord.c | 2 +- src/backend/utils/adt/ruleutils.c | 4 ++-- src/backend/utils/cache/catcache.c | 5 +---- src/pl/plpython/plpy_typeio.c | 4 ++-- 14 files changed, 25 insertions(+), 34 deletions(-) diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index 1e7696beb50..8bf8fc8ea2f 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -3020,7 +3020,7 @@ validate_pkattnums(Relation rel, for (j = 0; j < natts; j++) { /* dropped columns don't count */ - if (TupleDescAttr(tupdesc, j)->attisdropped) + if (TupleDescCompactAttr(tupdesc, j)->attisdropped) continue; if (++lnum == pkattnum) diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index a9a5671d95a..70564a68b13 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -1026,9 +1026,7 @@ check_selective_binary_conversion(RelOptInfo *baserel, numattrs = 0; for (i = 0; i < tupleDesc->natts; i++) { - Form_pg_attribute attr = TupleDescAttr(tupleDesc, i); - - if (attr->attisdropped) + if (TupleDescCompactAttr(tupleDesc, i)->attisdropped) continue; numattrs++; } diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index e5b5e1a5f51..f2fb0051843 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -1458,10 +1458,8 @@ deparseTargetList(StringInfo buf, first = true; for (i = 1; i <= tupdesc->natts; i++) { - Form_pg_attribute attr = TupleDescAttr(tupdesc, i - 1); - /* Ignore dropped attributes. */ - if (attr->attisdropped) + if (TupleDescCompactAttr(tupdesc, i - 1)->attisdropped) continue; if (have_wholerow || @@ -2150,7 +2148,7 @@ deparseInsertSql(StringInfo buf, RangeTblEntry *rte, foreach(lc, targetAttrs) { int attnum = lfirst_int(lc); - Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); + CompactAttribute *attr = TupleDescCompactAttr(tupdesc, attnum - 1); if (!first) appendStringInfoString(buf, ", "); @@ -2216,7 +2214,7 @@ rebuildInsertSql(StringInfo buf, Relation rel, foreach(lc, target_attrs) { int attnum = lfirst_int(lc); - Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); + CompactAttribute *attr = TupleDescCompactAttr(tupdesc, attnum - 1); if (!first) appendStringInfoString(buf, ", "); @@ -2266,7 +2264,7 @@ deparseUpdateSql(StringInfo buf, RangeTblEntry *rte, foreach(lc, targetAttrs) { int attnum = lfirst_int(lc); - Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); + CompactAttribute *attr = TupleDescCompactAttr(tupdesc, attnum - 1); if (!first) appendStringInfoString(buf, ", "); diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index 1d3ab22556d..3d71b442aa9 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -499,7 +499,7 @@ ginFlushBuildState(GinBuildState *buildstate, Relation index) &attnum, &key, &category, &nlist)) != NULL) { /* information about the key */ - Form_pg_attribute attr = TupleDescAttr(tdesc, (attnum - 1)); + CompactAttribute *attr = TupleDescCompactAttr(tdesc, (attnum - 1)); /* GIN tuple and tuple length */ GinTuple *tup; diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index c6de04819f1..5979580139f 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -3248,7 +3248,6 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmod Datum d; bool isNull; Datum conResult; - Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); d = slot_getattr(slot, attnum, &isNull); @@ -3261,6 +3260,8 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmod if (!isNull && !DatumGetBool(conResult)) { + Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); + /* * In principle the auxiliary information for this error * should be errdomainconstraint(), but errtablecol() diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index da5d901ec3c..f4b7343dace 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -2509,7 +2509,7 @@ get_dependent_generated_columns(PlannerInfo *root, Index rti, Bitmapset *attrs_used = NULL; /* skip if not generated column */ - if (!TupleDescAttr(tupdesc, defval->adnum - 1)->attgenerated) + if (!TupleDescCompactAttr(tupdesc, defval->adnum - 1)->attgenerated) continue; /* identify columns this generated column depends on */ diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 04ecf64b1fc..3c80bf1b9ce 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -3489,13 +3489,13 @@ get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum) if (tupdesc) { /* Composite data type, e.g. a table's row type */ - Form_pg_attribute att_tup; + CompactAttribute *att; Assert(tupdesc); Assert(attnum - atts_done <= tupdesc->natts); - att_tup = TupleDescAttr(tupdesc, - attnum - atts_done - 1); - return att_tup->attisdropped; + att = TupleDescCompactAttr(tupdesc, + attnum - atts_done - 1); + return att->attisdropped; } /* Otherwise, it can't have any dropped columns */ return false; diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index a54434151de..b57aef9916d 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -5134,7 +5134,7 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, for (natt = 0; natt < desc->natts; natt++) { - Form_pg_attribute attr = TupleDescAttr(desc, natt); + CompactAttribute *attr = TupleDescCompactAttr(desc, natt); ReorderBufferToastEnt *ent; struct varlena *varlena; @@ -5146,10 +5146,6 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, dlist_iter it; Size data_done = 0; - /* system columns aren't toasted */ - if (attr->attnum < 0) - continue; - if (attr->attisdropped) continue; diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 3c58ad88476..ec65a385f2d 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -975,9 +975,10 @@ slot_fill_defaults(LogicalRepRelMapEntry *rel, EState *estate, Assert(rel->attrmap->maplen == num_phys_attrs); for (attnum = 0; attnum < num_phys_attrs; attnum++) { + CompactAttribute *cattr = TupleDescCompactAttr(desc, attnum); Expr *defexpr; - if (TupleDescAttr(desc, attnum)->attisdropped || TupleDescAttr(desc, attnum)->attgenerated) + if (cattr->attisdropped || cattr->attgenerated) continue; if (rel->attrmap->attnums[attnum] >= 0) @@ -2833,7 +2834,7 @@ apply_handle_update(StringInfo s) target_perminfo = list_nth(estate->es_rteperminfos, 0); for (int i = 0; i < remoteslot->tts_tupleDescriptor->natts; i++) { - Form_pg_attribute att = TupleDescAttr(remoteslot->tts_tupleDescriptor, i); + CompactAttribute *att = TupleDescCompactAttr(remoteslot->tts_tupleDescriptor, i); int remoteattnum = rel->attrmap->attnums[i]; if (!att->attisdropped && remoteattnum >= 0) diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 847806b0a2e..378b61fbd18 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -1069,7 +1069,7 @@ check_and_init_gencol(PGOutputData *data, List *publications, /* Check if there is any generated column present. */ for (int i = 0; i < desc->natts; i++) { - Form_pg_attribute att = TupleDescAttr(desc, i); + CompactAttribute *att = TupleDescCompactAttr(desc, i); if (att->attgenerated) { diff --git a/src/backend/utils/adt/expandedrecord.c b/src/backend/utils/adt/expandedrecord.c index 13752db44e8..495d48fb581 100644 --- a/src/backend/utils/adt/expandedrecord.c +++ b/src/backend/utils/adt/expandedrecord.c @@ -547,7 +547,7 @@ expanded_record_set_tuple(ExpandedRecordHeader *erh, for (i = 0; i < erh->nfields; i++) { if (!erh->dnulls[i] && - !(TupleDescAttr(tupdesc, i)->attbyval)) + !(TupleDescCompactAttr(tupdesc, i)->attbyval)) { char *oldValue = (char *) DatumGetPointer(erh->dvalues[i]); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 050eef97a4c..79ec136231b 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9935,7 +9935,7 @@ get_rule_expr(Node *node, deparse_context *context, Node *e = (Node *) lfirst(arg); if (tupdesc == NULL || - !TupleDescAttr(tupdesc, i)->attisdropped) + !TupleDescCompactAttr(tupdesc, i)->attisdropped) { appendStringInfoString(buf, sep); /* Whole-row Vars need special treatment here */ @@ -9948,7 +9948,7 @@ get_rule_expr(Node *node, deparse_context *context, { while (i < tupdesc->natts) { - if (!TupleDescAttr(tupdesc, i)->attisdropped) + if (!TupleDescCompactAttr(tupdesc, i)->attisdropped) { appendStringInfoString(buf, sep); appendStringInfoString(buf, "NULL"); diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index fd1c7be8f53..509d9c6c7b4 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -2286,14 +2286,11 @@ CatCacheFreeKeys(TupleDesc tupdesc, int nkeys, int *attnos, Datum *keys) for (i = 0; i < nkeys; i++) { int attnum = attnos[i]; - Form_pg_attribute att; /* system attribute are not supported in caches */ Assert(attnum > 0); - att = TupleDescAttr(tupdesc, attnum - 1); - - if (!att->attbyval) + if (!TupleDescCompactAttr(tupdesc, attnum - 1)->attbyval) pfree(DatumGetPointer(keys[i])); } } diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c index bba78fb7592..d88d10068f3 100644 --- a/src/pl/plpython/plpy_typeio.c +++ b/src/pl/plpython/plpy_typeio.c @@ -1426,7 +1426,7 @@ PLySequence_ToComposite(PLyObToDatum *arg, TupleDesc desc, PyObject *sequence) idx = 0; for (i = 0; i < desc->natts; i++) { - if (!TupleDescAttr(desc, i)->attisdropped) + if (!TupleDescCompactAttr(desc, i)->attisdropped) idx++; } if (PySequence_Length(sequence) != idx) @@ -1443,7 +1443,7 @@ PLySequence_ToComposite(PLyObToDatum *arg, TupleDesc desc, PyObject *sequence) PyObject *volatile value; PLyObToDatum *att; - if (TupleDescAttr(desc, i)->attisdropped) + if (TupleDescCompactAttr(desc, i)->attisdropped) { values[i] = (Datum) 0; nulls[i] = true; -- cgit v1.2.3