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

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 1e7696beb5089a1f0106b6d0beb10c6ff68f55d6..8bf8fc8ea2f3b3aa92f38d7a99341b0c9ced1d8b 100644 (file)
--- 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 a9a5671d95a6fe85eb183504b96efd602108e9c6..70564a68b137b6aae412bb08ad423d4c5881a4f7 100644 (file)
--- 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 e5b5e1a5f51a5ce11eaa034af7d75658ac82c4bb..f2fb0051843169c28c39453e520f0a398876014b 100644 (file)
--- 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 1d3ab22556d89506bc614f780a9235da8dd437ef..3d71b442aa9e71da789cc79058d717f590818229 100644 (file)
--- 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 c6de04819f1747aa987097ac52ea9515550cae75..5979580139fc616e64cc7ec7d5cc4584ce90c9e3 100644 (file)
--- 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 da5d901ec3cab2adc6ec874978ad34047615e655..f4b7343daced66e12cd08fb856f91fa869897cce 100644 (file)
--- 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 04ecf64b1fc25002c9d620f521461063fff26de5..3c80bf1b9ce50fc62b1bd5799c6c366f7c4a5db0 100644 (file)
--- 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 a54434151debfb41d42f040ba4167527ee467d9e..b57aef9916def0f85aa06eb870c8498a216e5715 100644 (file)
--- 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 3c58ad884762fa3fb4a4c54f1a786383c4d256c0..ec65a385f2de00a63767b935bf35952022bfcded 100644 (file)
--- 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 847806b0a2e919fc68fd797647a8f370b547871c..378b61fbd188bb1fe93de8cb84034ba2f2133132 100644 (file)
--- 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 13752db44e8398b5a7cab5c9e112b384a75ca21f..495d48fb581b5ee659f6215d872364076a68afa8 100644 (file)
--- 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 050eef97a4cdc65c13a69b65139053b9b6f1bb1c..79ec136231be27bcf8642b73870ba87bd1bae757 100644 (file)
--- 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 fd1c7be8f53d884d0d9035f078b8e0f53749ee4d..509d9c6c7b4bff1705d131b35f92d46bcf38ea71 100644 (file)
--- 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 bba78fb75926b388833fe22ddcdc7d930ad8aede..d88d10068f3a754688b497af89a1b52647432631 100644 (file)
--- 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;