Replace the usage of heap_addheader to create pg_attribute tuples with regular

heap_form_tuple.  Since this removes the last remaining caller of
heap_addheader, remove it.

Extracted from the column privileges patch from Stephen Frost, with further
code cleanups by me.

Author: alvherre

src/backend/access/common/heaptuple.c

@@ -50,7 +50,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/common/heaptuple.c,v 1.123 2008/11/02 01:45:26 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/common/heaptuple.c,v 1.124 2008/11/14 01:57:41 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1576,56 +1576,3 @@ minimal_tuple_from_heap_tuple(HeapTuple htup)
 	result->t_len = len;
 	return result;
 }
-
-
-/* ----------------
- *		heap_addheader
- *
- * This routine forms a HeapTuple by copying the given structure (tuple
- * data) and adding a generic header.  Note that the tuple data is
- * presumed to contain no null fields and no varlena fields.
- *
- * This routine is really only useful for certain system tables that are
- * known to be fixed-width and null-free.  Currently it is only used for
- * pg_attribute tuples.
- * ----------------
- */
-HeapTuple
-heap_addheader(int natts,		/* max domain index */
-			   bool withoid,	/* reserve space for oid */
-			   Size structlen,	/* its length */
-			   void *structure) /* pointer to the struct */
-{
-	HeapTuple	tuple;
-	HeapTupleHeader td;
-	Size		len;
-	int			hoff;
-
-	AssertArg(natts > 0);
-
-	/* header needs no null bitmap */
-	hoff = offsetof(HeapTupleHeaderData, t_bits);
-	if (withoid)
-		hoff += sizeof(Oid);
-	hoff = MAXALIGN(hoff);
-	len = hoff + structlen;
-
-	tuple = (HeapTuple) palloc0(HEAPTUPLESIZE + len);
-	tuple->t_data = td = (HeapTupleHeader) ((char *) tuple + HEAPTUPLESIZE);
-
-	tuple->t_len = len;
-	ItemPointerSetInvalid(&(tuple->t_self));
-	tuple->t_tableOid = InvalidOid;
-
-	/* we don't bother to fill the Datum fields */
-
-	HeapTupleHeaderSetNatts(td, natts);
-	td->t_hoff = hoff;
-
-	if (withoid)				/* else leave infomask = 0 */
-		td->t_infomask = HEAP_HASOID;
-
-	memcpy((char *) td + hoff, structure, structlen);
-
-	return tuple;
-}

src/backend/catalog/heap.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.343 2008/11/09 21:24:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.344 2008/11/14 01:57:41 alvherre Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -478,6 +478,60 @@ CheckAttributeType(const char *attname, Oid atttypid)
 	}
 }
 
+/*
+ * InsertPgAttributeTuple
+ *		Construct and insert a new tuple in pg_attribute.
+ *
+ * Caller has already opened and locked pg_attribute.  new_attribute is the
+ * attribute to insert.
+ *
+ * indstate is the index state for CatalogIndexInsert.  It can be passed as
+ * NULL, in which case we'll fetch the necessary info.  (Don't do this when
+ * inserting multiple attributes, because it's a tad more expensive.)
+ */
+void
+InsertPgAttributeTuple(Relation pg_attribute_rel,
+					   Form_pg_attribute new_attribute,
+					   CatalogIndexState indstate)
+{
+	Datum		values[Natts_pg_attribute];
+	bool		nulls[Natts_pg_attribute];
+	HeapTuple	tup;
+
+	/* This is a tad tedious, but way cleaner than what we used to do... */
+	memset(values, 0, sizeof(values));
+	memset(nulls, false, sizeof(nulls));
+
+	values[Anum_pg_attribute_attrelid - 1] = ObjectIdGetDatum(new_attribute->attrelid);
+	values[Anum_pg_attribute_attname - 1] = NameGetDatum(&new_attribute->attname);
+	values[Anum_pg_attribute_atttypid - 1] = ObjectIdGetDatum(new_attribute->atttypid);
+	values[Anum_pg_attribute_attstattarget - 1] = Int32GetDatum(new_attribute->attstattarget);
+	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_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);
+	values[Anum_pg_attribute_attalign - 1] = CharGetDatum(new_attribute->attalign);
+	values[Anum_pg_attribute_attnotnull - 1] = BoolGetDatum(new_attribute->attnotnull);
+	values[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(new_attribute->atthasdef);
+	values[Anum_pg_attribute_attisdropped - 1] = BoolGetDatum(new_attribute->attisdropped);
+	values[Anum_pg_attribute_attislocal - 1] = BoolGetDatum(new_attribute->attislocal);
+	values[Anum_pg_attribute_attinhcount - 1] = Int32GetDatum(new_attribute->attinhcount);
+
+	tup = heap_form_tuple(RelationGetDescr(pg_attribute_rel), values, nulls);
+
+	/* finally insert the new tuple, update the indexes, and clean up */
+	simple_heap_insert(pg_attribute_rel, tup);
+
+	if (indstate != NULL)
+		CatalogIndexInsert(indstate, tup);
+	else
+		CatalogUpdateIndexes(pg_attribute_rel, tup);
+
+	heap_freetuple(tup);
+}
 /* --------------------------------
  *		AddNewAttributeTuples
  *
@@ -492,9 +546,8 @@ AddNewAttributeTuples(Oid new_rel_oid,
 					  bool oidislocal,
 					  int oidinhcount)
 {
-	const Form_pg_attribute *dpp;
+	Form_pg_attribute attr;
 	int			i;
-	HeapTuple	tup;
 	Relation	rel;
 	CatalogIndexState indstate;
 	int			natts = tupdesc->natts;
@@ -512,35 +565,25 @@ AddNewAttributeTuples(Oid new_rel_oid,
 	 * First we add the user attributes.  This is also a convenient place to
 	 * add dependencies on their datatypes.
 	 */
-	dpp = tupdesc->attrs;
 	for (i = 0; i < natts; i++)
 	{
+		attr = tupdesc->attrs[i];
 		/* Fill in the correct relation OID */
-		(*dpp)->attrelid = new_rel_oid;
+		attr->attrelid = new_rel_oid;
 		/* Make sure these are OK, too */
-		(*dpp)->attstattarget = -1;
-		(*dpp)->attcacheoff = -1;
-
-		tup = heap_addheader(Natts_pg_attribute,
-							 false,
-							 ATTRIBUTE_TUPLE_SIZE,
-							 (void *) *dpp);
-
-		simple_heap_insert(rel, tup);
-
-		CatalogIndexInsert(indstate, tup);
+		attr->attstattarget = -1;
+		attr->attcacheoff = -1;
 
-		heap_freetuple(tup);
+		InsertPgAttributeTuple(rel, attr, indstate);
 
+		/* Add dependency info */
 		myself.classId = RelationRelationId;
 		myself.objectId = new_rel_oid;
 		myself.objectSubId = i + 1;
 		referenced.classId = TypeRelationId;
-		referenced.objectId = (*dpp)->atttypid;
+		referenced.objectId = attr->atttypid;
 		referenced.objectSubId = 0;
 		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
-
-		dpp++;
 	}
 
 	/*
@@ -550,43 +593,28 @@ AddNewAttributeTuples(Oid new_rel_oid,
 	 */
 	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
 	{
-		dpp = SysAtt;
-		for (i = 0; i < (int) lengthof(SysAtt); i++, dpp++)
+		for (i = 0; i < (int) lengthof(SysAtt); i++)
 		{
-			if (tupdesc->tdhasoid ||
-				(*dpp)->attnum != ObjectIdAttributeNumber)
-			{
-				Form_pg_attribute attStruct;
-
-				tup = heap_addheader(Natts_pg_attribute,
-									 false,
-									 ATTRIBUTE_TUPLE_SIZE,
-									 (void *) *dpp);
-				attStruct = (Form_pg_attribute) GETSTRUCT(tup);
-
-				/* Fill in the correct relation OID in the copied tuple */
-				attStruct->attrelid = new_rel_oid;
+			FormData_pg_attribute attStruct;
 
-				/* Fill in correct inheritance info for the OID column */
-				if (attStruct->attnum == ObjectIdAttributeNumber)
-				{
-					attStruct->attislocal = oidislocal;
-					attStruct->attinhcount = oidinhcount;
-				}
-
-				/*
-				 * Unneeded since they should be OK in the constant data
-				 * anyway
-				 */
-				/* attStruct->attstattarget = 0; */
-				/* attStruct->attcacheoff = -1; */
+			/* skip OID where appropriate */
+			if (!tupdesc->tdhasoid &&
+				SysAtt[i]->attnum == ObjectIdAttributeNumber)
+				continue;
 
-				simple_heap_insert(rel, tup);
+			memcpy(&attStruct, (char *) SysAtt[i], sizeof(FormData_pg_attribute));
 
-				CatalogIndexInsert(indstate, tup);
+			/* Fill in the correct relation OID in the copied tuple */
+			attStruct.attrelid = new_rel_oid;
 
-				heap_freetuple(tup);
+			/* Fill in correct inheritance info for the OID column */
+			if (attStruct.attnum == ObjectIdAttributeNumber)
+			{
+				attStruct.attislocal = oidislocal;
+				attStruct.attinhcount = oidinhcount;
 			}
+
+			InsertPgAttributeTuple(rel, &attStruct, indstate);
 		}
 	}
 

src/backend/catalog/index.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.308 2008/11/13 17:42:10 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.309 2008/11/14 01:57:41 alvherre Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -327,7 +327,6 @@ AppendAttributeTuples(Relation indexRelation, int numatts)
 	Relation	pg_attribute;
 	CatalogIndexState indstate;
 	TupleDesc	indexTupDesc;
-	HeapTuple	new_tuple;
 	int			i;
 
 	/*
@@ -351,16 +350,7 @@ AppendAttributeTuples(Relation indexRelation, int numatts)
 		Assert(indexTupDesc->attrs[i]->attnum == i + 1);
 		Assert(indexTupDesc->attrs[i]->attcacheoff == -1);
 
-		new_tuple = heap_addheader(Natts_pg_attribute,
-								   false,
-								   ATTRIBUTE_TUPLE_SIZE,
-								   (void *) indexTupDesc->attrs[i]);
-
-		simple_heap_insert(pg_attribute, new_tuple);
-
-		CatalogIndexInsert(indstate, new_tuple);
-
-		heap_freetuple(new_tuple);
+		InsertPgAttributeTuple(pg_attribute, indexTupDesc->attrs[i], indstate);
 	}
 
 	CatalogCloseIndexes(indstate);

src/backend/commands/tablecmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.269 2008/11/02 01:45:27 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.270 2008/11/14 01:57:41 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3450,9 +3450,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
 	Relation	pgclass,
 				attrdesc;
 	HeapTuple	reltup;
-	HeapTuple	attributeTuple;
-	Form_pg_attribute attribute;
-	FormData_pg_attribute attributeD;
+	FormData_pg_attribute attribute;
 	int			i;
 	int			minattnum,
 				maxatts;
@@ -3543,37 +3541,27 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
 	/* make sure datatype is legal for a column */
 	CheckAttributeType(colDef->colname, typeOid);
 
-	attributeTuple = heap_addheader(Natts_pg_attribute,
-									false,
-									ATTRIBUTE_TUPLE_SIZE,
-									(void *) &attributeD);
-
-	attribute = (Form_pg_attribute) GETSTRUCT(attributeTuple);
-
-	attribute->attrelid = myrelid;
-	namestrcpy(&(attribute->attname), colDef->colname);
-	attribute->atttypid = typeOid;
-	attribute->attstattarget = -1;
-	attribute->attlen = tform->typlen;
-	attribute->attcacheoff = -1;
-	attribute->atttypmod = typmod;
-	attribute->attnum = i;
-	attribute->attbyval = tform->typbyval;
-	attribute->attndims = list_length(colDef->typename->arrayBounds);
-	attribute->attstorage = tform->typstorage;
-	attribute->attalign = tform->typalign;
-	attribute->attnotnull = colDef->is_not_null;
-	attribute->atthasdef = false;
-	attribute->attisdropped = false;
-	attribute->attislocal = colDef->is_local;
-	attribute->attinhcount = colDef->inhcount;
+	attribute.attrelid = myrelid;
+	namestrcpy(&(attribute.attname), colDef->colname);
+	attribute.atttypid = typeOid;
+	attribute.attstattarget = -1;
+	attribute.attlen = tform->typlen;
+	attribute.attcacheoff = -1;
+	attribute.atttypmod = typmod;
+	attribute.attnum = i;
+	attribute.attbyval = tform->typbyval;
+	attribute.attndims = list_length(colDef->typename->arrayBounds);
+	attribute.attstorage = tform->typstorage;
+	attribute.attalign = tform->typalign;
+	attribute.attnotnull = colDef->is_not_null;
+	attribute.atthasdef = false;
+	attribute.attisdropped = false;
+	attribute.attislocal = colDef->is_local;
+	attribute.attinhcount = colDef->inhcount;
 
 	ReleaseSysCache(typeTuple);
 
-	simple_heap_insert(attrdesc, attributeTuple);
-
-	/* Update indexes on pg_attribute */
-	CatalogUpdateIndexes(attrdesc, attributeTuple);
+	InsertPgAttributeTuple(attrdesc, &attribute, NULL);
 
 	heap_close(attrdesc, RowExclusiveLock);
 
@@ -3602,7 +3590,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
 		RawColumnDefault *rawEnt;
 
 		rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault));
-		rawEnt->attnum = attribute->attnum;
+		rawEnt->attnum = attribute.attnum;
 		rawEnt->raw_default = copyObject(colDef->raw_default);
 
 		/*
@@ -3637,7 +3625,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
 	 * returned by AddRelationNewConstraints, so that the right thing happens
 	 * when a datatype's default applies.
 	 */
-	defval = (Expr *) build_column_default(rel, attribute->attnum);
+	defval = (Expr *) build_column_default(rel, attribute.attnum);
 
 	if (!defval && GetDomainConstraints(typeOid) != NIL)
 	{
@@ -3664,7 +3652,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
 		NewColumnValue *newval;
 
 		newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
-		newval->attnum = attribute->attnum;
+		newval->attnum = attribute.attnum;
 		newval->expr = defval;
 
 		tab->newvals = lappend(tab->newvals, newval);
@@ -3678,7 +3666,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
 	/*
 	 * Add needed dependency entries for the new column.
 	 */
-	add_column_datatype_dependency(myrelid, i, attribute->atttypid);
+	add_column_datatype_dependency(myrelid, i, attribute.atttypid);
 }
 
 /*