From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 3 Jan 2018 22:53:06 +0000 (-0500)
Subject: Clean up tupdesc.c for recent changes.
X-Git-Tag: REL_11_BETA1~1002
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=47c6772eb7222dbfa200db4bbeba8002b96b7976;p=postgresql

Clean up tupdesc.c for recent changes.

TupleDescCopy needs to have the same effects as CreateTupleDescCopy in
that, since it doesn't copy constraints, it should clear the per-attribute
fields associated with them.  Oversight in commit cc5f81366.

Since TupleDescCopy has already established the presumption that it
can just flat-copy the entire attribute array in one go, propagate
that approach into CreateTupleDescCopy and CreateTupleDescCopyConstr.
(I'm suspicious that this would lead to valgrind complaints if we
had any trailing padding in the struct, but we do not, and anyway
fixing that seems like a job for a separate commit.)

Add some better comments.

Thomas Munro, reviewed by Vik Fearing, some additional hacking by me

Discussion: https://postgr.es/m/CAEepm=0NvOGZ8B6GbQyQe2C_c2m3LKJ9w=8OMBaYRLgZ_Gw6Nw@mail.gmail.com
---

diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index f07129ac53..f1f44230cd 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -52,6 +52,14 @@ CreateTemplateTupleDesc(int natts, bool hasoid)
 	/*
 	 * Allocate enough memory for the tuple descriptor, including the
 	 * attribute rows.
+	 *
+	 * Note: the attribute array stride is sizeof(FormData_pg_attribute),
+	 * since we declare the array elements as FormData_pg_attribute for
+	 * notational convenience.  However, we only guarantee that the first
+	 * ATTRIBUTE_FIXED_PART_SIZE bytes of each entry are valid; most code that
+	 * copies tupdesc entries around copies just that much.  In principle that
+	 * could be less due to trailing padding, although with the current
+	 * definition of pg_attribute there probably isn't any padding.
 	 */
 	desc = (TupleDesc) palloc(offsetof(struct tupleDesc, attrs) +
 							  natts * sizeof(FormData_pg_attribute));
@@ -106,16 +114,25 @@ CreateTupleDescCopy(TupleDesc tupdesc)
 
 	desc = CreateTemplateTupleDesc(tupdesc->natts, tupdesc->tdhasoid);
 
+	/* Flat-copy the attribute array */
+	memcpy(TupleDescAttr(desc, 0),
+		   TupleDescAttr(tupdesc, 0),
+		   desc->natts * sizeof(FormData_pg_attribute));
+
+	/*
+	 * Since we're not copying constraints and defaults, clear fields
+	 * associated with them.
+	 */
 	for (i = 0; i < desc->natts; i++)
 	{
 		Form_pg_attribute att = TupleDescAttr(desc, i);
 
-		memcpy(att, &tupdesc->attrs[i], ATTRIBUTE_FIXED_PART_SIZE);
 		att->attnotnull = false;
 		att->atthasdef = false;
 		att->attidentity = '\0';
 	}
 
+	/* We can copy the tuple type identification, too */
 	desc->tdtypeid = tupdesc->tdtypeid;
 	desc->tdtypmod = tupdesc->tdtypmod;
 
@@ -136,13 +153,12 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
 
 	desc = CreateTemplateTupleDesc(tupdesc->natts, tupdesc->tdhasoid);
 
-	for (i = 0; i < desc->natts; i++)
-	{
-		memcpy(TupleDescAttr(desc, i),
-			   TupleDescAttr(tupdesc, i),
-			   ATTRIBUTE_FIXED_PART_SIZE);
-	}
+	/* Flat-copy the attribute array */
+	memcpy(TupleDescAttr(desc, 0),
+		   TupleDescAttr(tupdesc, 0),
+		   desc->natts * sizeof(FormData_pg_attribute));
 
+	/* Copy the TupleConstr data structure, if any */
 	if (constr)
 	{
 		TupleConstr *cpy = (TupleConstr *) palloc0(sizeof(TupleConstr));
@@ -178,6 +194,7 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
 		desc->constr = cpy;
 	}
 
+	/* We can copy the tuple type identification, too */
 	desc->tdtypeid = tupdesc->tdtypeid;
 	desc->tdtypmod = tupdesc->tdtypmod;
 
@@ -195,8 +212,29 @@ CreateTupleDescCopyConstr(TupleDesc tupdesc)
 void
 TupleDescCopy(TupleDesc dst, TupleDesc src)
 {
+	int			i;
+
+	/* Flat-copy the header and attribute array */
 	memcpy(dst, src, TupleDescSize(src));
+
+	/*
+	 * Since we're not copying constraints and defaults, clear fields
+	 * associated with them.
+	 */
+	for (i = 0; i < dst->natts; i++)
+	{
+		Form_pg_attribute att = TupleDescAttr(dst, i);
+
+		att->attnotnull = false;
+		att->atthasdef = false;
+		att->attidentity = '\0';
+	}
 	dst->constr = NULL;
+
+	/*
+	 * Also, assume the destination is not to be ref-counted.  (Copying the
+	 * source's refcount would be wrong in any case.)
+	 */
 	dst->tdrefcount = -1;
 }