Recommend wrappers of PG_DETOAST_DATUM_PACKED()

Started by Noah Mischalmost 10 years ago3 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t35496
psql -h localhost -U postgres

Built from patchset v3 (message #3), July 28, 2026 at 08:08 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t35496_3 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t35496_3 && git checkout t35496_3

Patchset v3 (message #3) is on t35496_3

Jump to latest
#1Noah Misch
noah@leadboat.com

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

Attachments:

unpack-char-varlena-deprecate-v2.patchtext/plain; charset=us-asciiDownload+27-22
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noah Misch (#1)
Re: Recommend wrappers of PG_DETOAST_DATUM_PACKED()

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

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

#3Noah Misch
noah@leadboat.com
In reply to: Noah Misch (#1)
Re: Recommend wrappers of PG_DETOAST_DATUM_PACKED()

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

Attachments:

t35496_3
pp1-varlena-sz-v1.patchtext/plain; charset=us-asciiDownload+1-1
pp2-deconstruct_array-v1.patchtext/plain; charset=us-asciiDownload+33-30
pp3-adminpack-v1.patchtext/plain; charset=us-asciiDownload+7-12
pp4-unpack-char-varlena-deprecate-v3.patchtext/plain; charset=us-asciiDownload+76-59
pp5-backfill-v1.patchtext/plain; charset=us-asciiDownload+521-529