Re: Polyphase merge is obsolete
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
John Naylor <john.naylor@enterprisedb.com>
Cc: Jaime Casanova <jcasanov@systemguards.com.ec>,
vignesh C <vignesh21@gmail.com>, pgsql-hackers
<pgsql-hackers@postgresql.org>, Peter Geoghegan <pg@bowt.ie>,
Tomas Vondra <tomas.vondra@2ndquadrant.com>, Zhihong Yu <zyu@yugabyte.com>,
Jeff Davis <pgsql@j-davis.com>
Date: 2022-11-20T23:57:20Z
Lists: pgsql-hackers
Attachments
- 0001-Fix-and-clarify-function-comment-on-LogicalTapeSetCr.patch (text/x-patch) patch 0001
On 19/11/2022 13:00, Peter Eisentraut wrote: > On 18.10.21 14:15, Heikki Linnakangas wrote: >> On 05/10/2021 20:24, John Naylor wrote: >>> I've had a chance to review and test out the v5 patches. >> >> Thanks! I fixed the stray reference to PostgreSQL 14 that Zhihong >> mentioned, and pushed. > > AFAICT, this thread updated the API of LogicalTapeSetCreate() in PG15, > but did not adequately update the function header comment. The comment > still mentions the "shared" argument, which has been removed. There is > a new "preallocate" argument that is not mentioned at all. Also, it's > not easy to match the actual callers to the call variants described in > the comment. Could someone who remembers this work perhaps look this > over and update the comment? Is the attached more readable? I'm not 100% sure of the "preallocate" argument. If I understand correctly, you should pass true if you are writing multiple tapes at the same time, and false otherwise. HashAgg passed true, tuplesort passes false. However, it's not clear to me why we couldn't just always do the preallocation. It seems pretty harmless even if it's not helpful. Or do it when there are multiple writer tapes, and not otherwise. The parameter was added in commit 0758964963 so presumably there was a reason, but at a quick glance at the thread that led to that commit, I couldn't see what it was. - Heikki
Commits
-
Fix and clarify function comment on LogicalTapeSetCreate.
- 95f62b16a334 15.2 landed
- 236f1ea84c4c 16.0 landed
-
Refactor LogicalTapeSet/LogicalTape interface.
- c4649cce39a4 15.0 landed
-
Replace polyphase merge algorithm with a simple balanced k-way merge.
- 65014000b351 15.0 landed
-
logtape.c: do not preallocate for tapes when sorting
- 075896496394 14.0 cited