Recommend wrappers of PG_DETOAST_DATUM_PACKED().

When commit 3e23b68 introduced
single-byte varlena headers, its fmgr.h changes presented
PG_GETARG_TEXT_PP() and PG_GETARG_TEXT_P() as equals.  Its postgres.h
changes presented PG_DETOAST_DATUM_PACKED() and VARDATA_ANY() as the
exceptional case.  Now, instead, firmly recommend PG_GETARG_TEXT_PP()
over PG_GETARG_TEXT_P(); likewise for other ...PP() macros.  This shaves
cycles and invites consistency of style.

Author: nmisch

doc/src/sgml/xfunc.sgml

@@ -2388,18 +2388,23 @@ PG_FUNCTION_INFO_V1(copytext);
 Datum
 copytext(PG_FUNCTION_ARGS)
 {
-    text     *t = PG_GETARG_TEXT_P(0);
+    text     *t = PG_GETARG_TEXT_PP(0);
+
     /*
-     * VARSIZE is the total size of the struct in bytes.
+     * VARSIZE_ANY_EXHDR is the size of the struct in bytes, minus the
+     * VARHDRSZ or VARHDRSZ_SHORT of its header.  Construct the copy with a
+     * full-length header.
      */
-    text     *new_t = (text *) palloc(VARSIZE(t));
-    SET_VARSIZE(new_t, VARSIZE(t));
+    text     *new_t = (text *) palloc(VARSIZE_ANY_EXHDR(t) + VARHDRSZ);
+    SET_VARSIZE(new_t, VARSIZE_ANY_EXHDR(t) + VARHDRSZ);
+
     /*
-     * VARDATA is a pointer to the data region of the struct.
+     * VARDATA is a pointer to the data region of the new struct.  The source
+     * could be a short datum, so retrieve its data through VARDATA_ANY.
      */
     memcpy((void *) VARDATA(new_t), /* destination */
-           (void *) VARDATA(t),     /* source */
-           VARSIZE(t) - VARHDRSZ);  /* how many bytes */
+           (void *) VARDATA_ANY(t), /* source */
+           VARSIZE_ANY_EXHDR(t));   /* how many bytes */
     PG_RETURN_TEXT_P(new_t);
 }
 
@@ -2408,15 +2413,16 @@ PG_FUNCTION_INFO_V1(concat_text);
 Datum
 concat_text(PG_FUNCTION_ARGS)
 {
-    text  *arg1 = PG_GETARG_TEXT_P(0);
-    text  *arg2 = PG_GETARG_TEXT_P(1);
-    int32 new_text_size = VARSIZE(arg1) + VARSIZE(arg2) - VARHDRSZ;
+    text  *arg1 = PG_GETARG_TEXT_PP(0);
+    text  *arg2 = PG_GETARG_TEXT_PP(1);
+    int32 arg1_size = VARSIZE_ANY_EXHDR(arg1);
+    int32 arg2_size = VARSIZE_ANY_EXHDR(arg2);
+    int32 new_text_size = arg1_size + arg2_size + VARHDRSZ;
     text *new_text = (text *) palloc(new_text_size);
 
     SET_VARSIZE(new_text, new_text_size);
-    memcpy(VARDATA(new_text), VARDATA(arg1), VARSIZE(arg1) - VARHDRSZ);
-    memcpy(VARDATA(new_text) + (VARSIZE(arg1) - VARHDRSZ),
-           VARDATA(arg2), VARSIZE(arg2) - VARHDRSZ);
+    memcpy(VARDATA(new_text), VARDATA_ANY(arg1), arg1_size);
+    memcpy(VARDATA(new_text) + arg1_size, VARDATA_ANY(arg2), arg2_size);
     PG_RETURN_TEXT_P(new_text);
 }
 ]]>

src/include/c.h

@@ -429,10 +429,11 @@ typedef struct
  * may be compressed or moved out-of-line.  However datatype-specific routines
  * are mostly content to deal with de-TOASTed values only, and of course
  * client-side routines should never see a TOASTed value.  But even in a
- * de-TOASTed value, beware of touching vl_len_ directly, as its representation
- * is no longer convenient.  It's recommended that code always use the VARDATA,
- * VARSIZE, and SET_VARSIZE macros instead of relying on direct mentions of
- * the struct fields.  See postgres.h for details of the TOASTed form.
+ * de-TOASTed value, beware of touching vl_len_ directly, as its
+ * representation is no longer convenient.  It's recommended that code always
+ * use macros VARDATA_ANY, VARSIZE_ANY, VARSIZE_ANY_EXHDR, VARDATA, VARSIZE,
+ * and SET_VARSIZE instead of relying on direct mentions of the struct fields.
+ * See postgres.h for details of the TOASTed form.
  * ----------------
  */
 struct varlena

src/include/fmgr.h

@@ -178,11 +178,12 @@ extern void fmgr_info_copy(FmgrInfo *dstinfo, FmgrInfo *srcinfo,
  * The resulting datum can be accessed using VARSIZE_ANY() and VARDATA_ANY()
  * (beware of multiple evaluations in those macros!)
  *
- * WARNING: It is only safe to use pg_detoast_datum_packed() and
- * VARDATA_ANY() if you really don't care about the alignment. Either because
- * you're working with something like text where the alignment doesn't matter
- * or because you're not going to access its constituent parts and just use
- * things like memcpy on it anyways.
+ * In consumers oblivious to data alignment, call PG_DETOAST_DATUM_PACKED(),
+ * VARDATA_ANY(), VARSIZE_ANY() and VARSIZE_ANY_EXHDR().  Elsewhere, call
+ * PG_DETOAST_DATUM(), VARDATA() and VARSIZE().  Directly fetching an int16,
+ * int32 or wider field in the struct representing the datum layout requires
+ * aligned data.  memcpy() is alignment-oblivious, as are most operations on
+ * datatypes, such as text, whose layout struct contains only char fields.
  *
  * Note: it'd be nice if these could be macros, but I see no way to do that
  * without evaluating the arguments multiple times, which is NOT acceptable.
@@ -243,13 +244,9 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena * datum);
 /* and this if you can handle 1-byte-header datums: */
 #define PG_GETARG_VARLENA_PP(n) PG_DETOAST_DATUM_PACKED(PG_GETARG_DATUM(n))
 /* DatumGetFoo macros for varlena types will typically look like this: */
-#define DatumGetByteaP(X)			((bytea *) PG_DETOAST_DATUM(X))
 #define DatumGetByteaPP(X)			((bytea *) PG_DETOAST_DATUM_PACKED(X))
-#define DatumGetTextP(X)			((text *) PG_DETOAST_DATUM(X))
 #define DatumGetTextPP(X)			((text *) PG_DETOAST_DATUM_PACKED(X))
-#define DatumGetBpCharP(X)			((BpChar *) PG_DETOAST_DATUM(X))
 #define DatumGetBpCharPP(X)			((BpChar *) PG_DETOAST_DATUM_PACKED(X))
-#define DatumGetVarCharP(X)			((VarChar *) PG_DETOAST_DATUM(X))
 #define DatumGetVarCharPP(X)		((VarChar *) PG_DETOAST_DATUM_PACKED(X))
 #define DatumGetHeapTupleHeader(X)	((HeapTupleHeader) PG_DETOAST_DATUM(X))
 /* And we also offer variants that return an OK-to-write copy */
@@ -264,13 +261,9 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena * datum);
 #define DatumGetBpCharPSlice(X,m,n) ((BpChar *) PG_DETOAST_DATUM_SLICE(X,m,n))
 #define DatumGetVarCharPSlice(X,m,n) ((VarChar *) PG_DETOAST_DATUM_SLICE(X,m,n))
 /* GETARG macros for varlena types will typically look like this: */
-#define PG_GETARG_BYTEA_P(n)		DatumGetByteaP(PG_GETARG_DATUM(n))
 #define PG_GETARG_BYTEA_PP(n)		DatumGetByteaPP(PG_GETARG_DATUM(n))
-#define PG_GETARG_TEXT_P(n)			DatumGetTextP(PG_GETARG_DATUM(n))
 #define PG_GETARG_TEXT_PP(n)		DatumGetTextPP(PG_GETARG_DATUM(n))
-#define PG_GETARG_BPCHAR_P(n)		DatumGetBpCharP(PG_GETARG_DATUM(n))
 #define PG_GETARG_BPCHAR_PP(n)		DatumGetBpCharPP(PG_GETARG_DATUM(n))
-#define PG_GETARG_VARCHAR_P(n)		DatumGetVarCharP(PG_GETARG_DATUM(n))
 #define PG_GETARG_VARCHAR_PP(n)		DatumGetVarCharPP(PG_GETARG_DATUM(n))
 #define PG_GETARG_HEAPTUPLEHEADER(n)	DatumGetHeapTupleHeader(PG_GETARG_DATUM(n))
 /* And we also offer variants that return an OK-to-write copy */
@@ -284,6 +277,21 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena * datum);
 #define PG_GETARG_TEXT_P_SLICE(n,a,b)  DatumGetTextPSlice(PG_GETARG_DATUM(n),a,b)
 #define PG_GETARG_BPCHAR_P_SLICE(n,a,b) DatumGetBpCharPSlice(PG_GETARG_DATUM(n),a,b)
 #define PG_GETARG_VARCHAR_P_SLICE(n,a,b) DatumGetVarCharPSlice(PG_GETARG_DATUM(n),a,b)
+/*
+ * Obsolescent variants that guarantee INT alignment for the return value.
+ * Few operations on these particular types need alignment, mainly operations
+ * that cast the VARDATA pointer to a type like int16[].  Most code should use
+ * the ...PP(X) counterpart.  Nonetheless, these appear frequently in code
+ * predating the PostgreSQL 8.3 introduction of the ...PP(X) variants.
+ */
+#define DatumGetByteaP(X)			((bytea *) PG_DETOAST_DATUM(X))
+#define DatumGetTextP(X)			((text *) PG_DETOAST_DATUM(X))
+#define DatumGetBpCharP(X)			((BpChar *) PG_DETOAST_DATUM(X))
+#define DatumGetVarCharP(X)			((VarChar *) PG_DETOAST_DATUM(X))
+#define PG_GETARG_BYTEA_P(n)		DatumGetByteaP(PG_GETARG_DATUM(n))
+#define PG_GETARG_TEXT_P(n)			DatumGetTextP(PG_GETARG_DATUM(n))
+#define PG_GETARG_BPCHAR_P(n)		DatumGetBpCharP(PG_GETARG_DATUM(n))
+#define PG_GETARG_VARCHAR_P(n)		DatumGetVarCharP(PG_GETARG_DATUM(n))
 
 /* To return a NULL do this: */
 #define PG_RETURN_NULL()  \

src/include/postgres.h

@@ -287,20 +287,18 @@ typedef struct
 /* Externally visible macros */
 
 /*
- * VARDATA, VARSIZE, and SET_VARSIZE are the recommended API for most code
- * for varlena datatypes.  Note that they only work on untoasted,
- * 4-byte-header Datums!
- *
- * Code that wants to use 1-byte-header values without detoasting should
- * use VARSIZE_ANY/VARSIZE_ANY_EXHDR/VARDATA_ANY.  The other macros here
- * should usually be used only by tuple assembly/disassembly code and
- * code that specifically wants to work with still-toasted Datums.
- *
- * WARNING: It is only safe to use VARDATA_ANY() -- typically with
- * PG_DETOAST_DATUM_PACKED() -- if you really don't care about the alignment.
- * Either because you're working with something like text where the alignment
- * doesn't matter or because you're not going to access its constituent parts
- * and just use things like memcpy on it anyways.
+ * In consumers oblivious to data alignment, call PG_DETOAST_DATUM_PACKED(),
+ * VARDATA_ANY(), VARSIZE_ANY() and VARSIZE_ANY_EXHDR().  Elsewhere, call
+ * PG_DETOAST_DATUM(), VARDATA() and VARSIZE().  Directly fetching an int16,
+ * int32 or wider field in the struct representing the datum layout requires
+ * aligned data.  memcpy() is alignment-oblivious, as are most operations on
+ * datatypes, such as text, whose layout struct contains only char fields.
+ *
+ * Code assembling a new datum should call VARDATA() and SET_VARSIZE().
+ * (Datums begin life untoasted.)
+ *
+ * Other macros here should usually be used only by tuple assembly/disassembly
+ * code and code that specifically wants to work with still-toasted Datums.
  */
 #define VARDATA(PTR)						VARDATA_4B(PTR)
 #define VARSIZE(PTR)						VARSIZE_4B(PTR)

src/include/utils/inet.h

@@ -104,12 +104,13 @@ typedef struct macaddr
 /*
  * fmgr interface macros
  */
-#define DatumGetInetP(X)	((inet *) PG_DETOAST_DATUM(X))
 #define DatumGetInetPP(X)	((inet *) PG_DETOAST_DATUM_PACKED(X))
 #define InetPGetDatum(X)	PointerGetDatum(X)
-#define PG_GETARG_INET_P(n) DatumGetInetP(PG_GETARG_DATUM(n))
 #define PG_GETARG_INET_PP(n) DatumGetInetPP(PG_GETARG_DATUM(n))
 #define PG_RETURN_INET_P(x) return InetPGetDatum(x)
+/* obsolescent variants */
+#define DatumGetInetP(X)	((inet *) PG_DETOAST_DATUM(X))
+#define PG_GETARG_INET_P(n) DatumGetInetP(PG_GETARG_DATUM(n))
 /* macaddr is a fixed-length pass-by-reference datatype */
 #define DatumGetMacaddrP(X)    ((macaddr *) DatumGetPointer(X))
 #define MacaddrPGetDatum(X)    PointerGetDatum(X)

src/tutorial/funcs_new.c

@@ -66,21 +66,24 @@ PG_FUNCTION_INFO_V1(copytext);
 Datum
 copytext(PG_FUNCTION_ARGS)
 {
-	text	   *t = PG_GETARG_TEXT_P(0);
+	text	   *t = PG_GETARG_TEXT_PP(0);
 
 	/*
-	 * VARSIZE is the total size of the struct in bytes.
+	 * VARSIZE_ANY_EXHDR is the size of the struct in bytes, minus the
+	 * VARHDRSZ or VARHDRSZ_SHORT of its header.  Construct the copy with a
+	 * full-length header.
 	 */
-	text	   *new_t = (text *) palloc(VARSIZE(t));
+	text	   *new_t = (text *) palloc(VARSIZE_ANY_EXHDR(t) + VARHDRSZ);
 
-	SET_VARSIZE(new_t, VARSIZE(t));
+	SET_VARSIZE(new_t, VARSIZE_ANY_EXHDR(t) + VARHDRSZ);
 
 	/*
-	 * VARDATA is a pointer to the data region of the struct.
+	 * VARDATA is a pointer to the data region of the new struct.  The source
+	 * could be a short datum, so retrieve its data through VARDATA_ANY.
 	 */
 	memcpy((void *) VARDATA(new_t),		/* destination */
-		   (void *) VARDATA(t), /* source */
-		   VARSIZE(t) - VARHDRSZ);		/* how many bytes */
+		   (void *) VARDATA_ANY(t),		/* source */
+		   VARSIZE_ANY_EXHDR(t));		/* how many bytes */
 	PG_RETURN_TEXT_P(new_t);
 }
 
@@ -89,16 +92,16 @@ PG_FUNCTION_INFO_V1(concat_text);
 Datum
 concat_text(PG_FUNCTION_ARGS)
 {
-	text	   *arg1 = PG_GETARG_TEXT_P(0);
-	text	   *arg2 = PG_GETARG_TEXT_P(1);
-	int32		arg1_size = VARSIZE(arg1) - VARHDRSZ;
-	int32		arg2_size = VARSIZE(arg2) - VARHDRSZ;
+	text	   *arg1 = PG_GETARG_TEXT_PP(0);
+	text	   *arg2 = PG_GETARG_TEXT_PP(1);
+	int32		arg1_size = VARSIZE_ANY_EXHDR(arg1);
+	int32		arg2_size = VARSIZE_ANY_EXHDR(arg2);
 	int32		new_text_size = arg1_size + arg2_size + VARHDRSZ;
 	text	   *new_text = (text *) palloc(new_text_size);
 
 	SET_VARSIZE(new_text, new_text_size);
-	memcpy(VARDATA(new_text), VARDATA(arg1), arg1_size);
-	memcpy(VARDATA(new_text) + arg1_size, VARDATA(arg2), arg2_size);
+	memcpy(VARDATA(new_text), VARDATA_ANY(arg1), arg1_size);
+	memcpy(VARDATA(new_text) + arg1_size, VARDATA_ANY(arg2), arg2_size);
 	PG_RETURN_TEXT_P(new_text);
 }