tuplesort_putdatum() does not account for tuple memory

Started by Mario Karuza6 days ago2 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.

appliessuccessCI 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:t253759
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 15, 2026 at 08:51 PM.

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 t253759_1 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 t253759_1 && git checkout t253759_1

Patchset v1 (message #1) is on t253759_1

Jump to latest
#1Mario Karuza
mkaruza.pg@icloud.com

Hi hackers,

Memory allocated for copied pass-by-reference Datums was not accounted
against work_mem because tuplesort_putdatum() passed a hardcoded tuplen
of 0 to tuplesort_puttuple_common(). Function free_sort_tuple() adjusts
the accounting by the amount actually allocated, so freeing such a
tuple subtracts an amount that was never added.

This was introduced in 6ed83d5fa55, which switched non-bounded sorts to
bump contexts. That commit correctly changed the other tuplesort_put*()
functions to compute the size, leaving only this one passing hardcoded
0.

So currently:

1) Bounded datum sorts are misreported. With work_mem = 4MB:

EXPLAIN ANALYZE SELECT md5(i::text) AS hash 
FROM generate_series(1,100000) i
ORDER BY hash LIMIT 5;

master: Sort Method: still in progress Memory: 0kB
patched: Sort Method: top-N heapsort Memory: 25kB

2) work_mem is not enforced against the tuple data, and hold more data
than allowed before spilling With work_mem = 4MB:

EXPLAIN ANALYZE SELECT md5(i::text) AS hash 
FROM generate_series(1,100000) i
ORDER BY hash;

master: Sort Method: quicksort Memory: 3073kB
patched: Sort Method: external merge Disk: 3920kB

The attached patch computes tuplen the way the tuplesort_put*()
variants do.

Thanks,
Mario

Attachments:

t253759_1
v1-0001-Fix-memory-accounting-for-datum-sorts-of-pass-by-.patchtext/x-patch; charset=UTF-8; name=v1-0001-Fix-memory-accounting-for-datum-sorts-of-pass-by-.patchDownload+11-2
#2David Rowley
dgrowleyml@gmail.com
In reply to: Mario Karuza (#1)
Re: tuplesort_putdatum() does not account for tuple memory

On Fri, 11 Sept 2026 at 08:36, Mario Karuza <mkaruza.pg@icloud.com> wrote:

Memory allocated for copied pass-by-reference Datums was not accounted
against work_mem because tuplesort_putdatum() passed a hardcoded tuplen
of 0 to tuplesort_puttuple_common(). Function free_sort_tuple() adjusts
the accounting by the amount actually allocated, so freeing such a
tuple subtracts an amount that was never added.

This was introduced in 6ed83d5fa55, which switched non-bounded sorts to
bump contexts. That commit correctly changed the other tuplesort_put*()
functions to compute the size, leaving only this one passing hardcoded
0.

I wondered if the extra strlen() call in datumGetSize() would result
in a slowdown, but looking deeper, it seems like that won't be hit due
to lack of sort support for cstring types.

I don't see any issues with the patch. I will process it on Monday, to
give time in case other people want to look.

David