Re: Recommend wrappers of PG_DETOAST_DATUM_PACKED()

From: Noah Misch <noah(at)leadboat(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Recommend wrappers of PG_DETOAST_DATUM_PACKED()
Date: 2017-02-27 08:12:17
Message-ID: 20170227081217.GA1521880@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
pp1-varlena-sz-v1.patch text/plain 835 bytes
pp2-deconstruct_array-v1.patch text/plain 7.1 KB
pp3-adminpack-v1.patch text/plain 1.7 KB
pp4-unpack-char-varlena-deprecate-v3.patch text/plain 13.3 KB
pp5-backfill-v1.patch text/plain 117.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2017-02-27 08:37:33 Re: Radix tree for character conversion
Previous Message Robert Haas 2017-02-27 07:54:34 Re: Creation of "Future" commit fest, named 2017-07