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 <[email protected]> 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 >
