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 <dgrowleyml@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAApHDvrMy90o1Lgkt31F82tcSuwRFHq3vyGewSRN=-QuSEEvyQ@mail.gmail.com

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)

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++;
 	}

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, ", ");

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;

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()

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 */

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;

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;
 

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)

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)
 		{

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]);
 

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");

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]));
 	}
 }

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;