commit 76a7bb0 Author: Noah Misch AuthorDate: Sun Feb 26 22:25:22 2017 -0500 Commit: Noah Misch CommitDate: Sun Feb 26 22:25:22 2017 -0500 Recommend wrappers of PG_DETOAST_DATUM_PACKED(). When commit 3e23b68dac006e8deb0afa327e855258df8de064 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. diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 255bfdd..94a7ad7 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/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); } ]]> diff --git a/src/include/c.h b/src/include/c.h index 69245c5..47e15df 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -426,10 +426,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 diff --git a/src/include/fmgr.h b/src/include/fmgr.h index a671480..4d400d2 100644 --- a/src/include/fmgr.h +++ b/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() \ diff --git a/src/include/postgres.h b/src/include/postgres.h index ff2c5c0..f3582d5 100644 --- a/src/include/postgres.h +++ b/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) diff --git a/src/include/utils/inet.h b/src/include/utils/inet.h index 577b34d..b4d7359 100644 --- a/src/include/utils/inet.h +++ b/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) diff --git a/src/tutorial/funcs_new.c b/src/tutorial/funcs_new.c index f668d28..2e09f8d 100644 --- a/src/tutorial/funcs_new.c +++ b/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); }