Postgres Pro
Downloads
Home   >   mailing lists

Thread: Recommend wrappers of PG_DETOAST_DATUM_PACKED()

Recommend wrappers of PG_DETOAST_DATUM_PACKED()

From
Noah Misch
Date:
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.  Let's
firmly recommend PG_GETARG_TEXT_PP() over PG_GETARG_TEXT_P(); likewise for
other ...PP() macros.  This shaves cycles and brings consistency of style.

If the attached policy-setting patch seems reasonable, I will follow it with a
mechanical patch adopting the recommended macros throughout the tree.  (If any
code does want the alignment it gets from an obsolecent macro, I will add a
comment to that code.)

Thanks,
nm
Attachment

Re: Recommend wrappers of PG_DETOAST_DATUM_PACKED()

From
Tom Lane
Date:
Noah Misch <noah@leadboat.com> writes:
> 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.  Let's
> firmly recommend PG_GETARG_TEXT_PP() over PG_GETARG_TEXT_P(); likewise for
> other ...PP() macros.  This shaves cycles and brings consistency of style.

+1
        regards, tom lane

Re: [HACKERS] Recommend wrappers of PG_DETOAST_DATUM_PACKED()

From
Noah Misch
Date:
On Tue, Oct 25, 2016 at 10:57:24AM -0400, Noah Misch wrote:
> 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.  Let's
> firmly recommend PG_GETARG_TEXT_PP() over PG_GETARG_TEXT_P(); likewise for
> other ...PP() macros.  This shaves cycles and brings consistency of style.
> 
> If the attached policy-setting patch seems reasonable, I will follow it with a
> mechanical patch adopting the recommended macros throughout the tree.  (If any
> code does want the alignment it gets from an obsolecent macro, I will add a
> comment to that code.)

That mechanical work unearthed special cases, hence the delay.  Patch series:

1. Fix old comment about length of text, bytea, etc.

2. Unify style around deconstruct_array().

3. Fix pg_file_write() error handling.  This is largely unrelated, but I
   noticed it when I was about to update the first VARSIZE() in that function.
   I'll back-patch this one.

4. Policy change.  This is a new version of the patch I posted upthread.  I
   added a recommendation of PG_GETARG_INET_PP() over PG_GETARG_INET_P().  I
   added updates to xfunc.sgml, src/tutorial, and a few more comments.

5. Use the now-preferred interfaces in more places.  The log message describes
   how I resolved some arguable points.  In particular, I declined to change
   areas where the detoasted varlena header leaks into longer-lived state.

Having stumbled on rijndael.c, I am not enormously confident that I noticed
all the places expecting alignment in bytea or text.  This patch series does
pass "make check-world" on a system where an unaligned 2B, 4B or 8B read
elicits SIGBUS.  If I did miss such an expectation, the result should be a
SIGBUS or a performance loss, not a silent malfunction.

Thanks,
nm

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachment