It seems to me that it would make sense if InsertPgAttributeTuple() were
to set attcacheoff to -1 instead of taking it from the caller.

InsertPgAttributeTuple() is the interface between in-memory tuple
descriptors and on-disk pg_attribute, so it makes sense to give it the
job of resetting attcacheoff.  This avoids having all the callers having
to do so.  There are also pending patches that have to work around this
in seemingly unnecessary ways.

(The comment about the "very grotty code" that I'm removing is from a
time when AppendAttributeTuples() would deform a tuple, reset
attcacheoff, then reform the tuple -- hence grotty.  This is long
obsolete, since the tuple is now formed later.)

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 9b19e5679b08d30c6fe40fd896b6a8d3dac45c24 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 17 Jul 2018 09:48:29 +0200
Subject: [PATCH] InsertPgAttributeTuple() to set attcacheoff

InsertPgAttributeTuple() is the interface between in-memory tuple
descriptors and on-disk pg_attribute, so it makes sense to give it the
job of resetting attcacheoff.  This avoids having all the callers having
to do so.
---
 src/backend/catalog/heap.c       | 9 ++++-----
 src/backend/catalog/index.c      | 5 -----
 src/backend/commands/tablecmds.c | 1 -
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 4cfc0c8911..ac5a677c5f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -592,8 +592,8 @@ CheckAttributeType(const char *attname,
  *             Construct and insert a new tuple in pg_attribute.
  *
  * Caller has already opened and locked pg_attribute.  new_attribute is the
- * attribute to insert (but we ignore attacl and attoptions, which are always
- * initialized to NULL).
+ * attribute to insert.  attcacheoff is always initialized to -1, attacl and
+ * attoptions are always initialized to NULL.
  *
  * 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
@@ -620,7 +620,7 @@ InsertPgAttributeTuple(Relation pg_attribute_rel,
        values[Anum_pg_attribute_attlen - 1] = 
Int16GetDatum(new_attribute->attlen);
        values[Anum_pg_attribute_attnum - 1] = 
Int16GetDatum(new_attribute->attnum);
        values[Anum_pg_attribute_attndims - 1] = 
Int32GetDatum(new_attribute->attndims);
-       values[Anum_pg_attribute_attcacheoff - 1] = 
Int32GetDatum(new_attribute->attcacheoff);
+       values[Anum_pg_attribute_attcacheoff - 1] = Int32GetDatum(-1);
        values[Anum_pg_attribute_atttypmod - 1] = 
Int32GetDatum(new_attribute->atttypmod);
        values[Anum_pg_attribute_attbyval - 1] = 
BoolGetDatum(new_attribute->attbyval);
        values[Anum_pg_attribute_attstorage - 1] = 
CharGetDatum(new_attribute->attstorage);
@@ -689,9 +689,8 @@ AddNewAttributeTuples(Oid new_rel_oid,
                attr = TupleDescAttr(tupdesc, i);
                /* Fill in the correct relation OID */
                attr->attrelid = new_rel_oid;
-               /* Make sure these are OK, too */
+               /* Make sure this is OK, too */
                attr->attstattarget = -1;
-               attr->attcacheoff = -1;
 
                InsertPgAttributeTuple(rel, attr, indstate);
 
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 2dad7b059e..b256054908 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -557,12 +557,7 @@ AppendAttributeTuples(Relation indexRelation, int numatts)
        {
                Form_pg_attribute attr = TupleDescAttr(indexTupDesc, i);
 
-               /*
-                * There used to be very grotty code here to set these fields, 
but I
-                * think it's unnecessary.  They should be set already.
-                */
                Assert(attr->attnum == i + 1);
-               Assert(attr->attcacheoff == -1);
 
                InsertPgAttributeTuple(pg_attribute, attr, indstate);
        }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f6210226e9..7cedc28c6b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5522,7 +5522,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
        attribute.atttypid = typeOid;
        attribute.attstattarget = (newattnum > 0) ? -1 : 0;
        attribute.attlen = tform->typlen;
-       attribute.attcacheoff = -1;
        attribute.atttypmod = typmod;
        attribute.attnum = newattnum;
        attribute.attbyval = tform->typbyval;
-- 
2.18.0

Reply via email to