Re: tuplesort_putdatum() does not account for tuple memory

Hi,

I tested this on master (50d6e533e4), on macOS arm64, with a debug
build with assertions enabled.

The patch applies cleanly and the regression tests pass. The two
EXPLAIN ANALYZE cases from the first message behave as described:

LIMIT query: still in progress / 0kB -> top-N heapsort / 25kB
plain ORDER BY: quicksort / 3073kB -> external merge / Disk:
3920kB

I also tried the other callers of tuplesort_putdatum() that go through
the datum path, using work_mem = 4MB, a 100k-row table of md5() values
and log_temp_files = 0 with log_statement = 'all'. On master,
count(DISTINCT h), array_agg(h ORDER BY h) and
percentile_disc(0.5) WITHIN GROUP (ORDER BY h) don't write any
temporary file. With the patch, each of them writes one (about 3.5MB),
so the bug was not limited to Sort nodes.

The new code follows what the other tuplesort_put*() functions do, and
the comment about GetMemoryChunkSpace() and bump contexts matches. I
have no concerns about the patch itself.

Two small questions: since 6ed83d5fa55 went into 17, are we planning
to backpatch this to 17 and 18? And since some queries that used to
stay in memory will now spill, would it be worth saying so in the
commit message?

Thanks,
João Marcelo

Em sex., 2 de out. de 2026 às 00:07, Mario Karuza <mkaruza(dot)pg(at)icloud(dot)com>
escreveu:

> 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
>