Re: Polyphase merge is obsolete

On 16/01/2023 12:23, Peter Eisentraut wrote:
> On 21.11.22 10:29, Peter Eisentraut wrote:
>> On 21.11.22 00:57, Heikki Linnakangas wrote:
>>> On 19/11/2022 13:00, Peter Eisentraut wrote:
>>>> 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?
>>
>> That looks better, thanks.
>>
>>> 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.
>>
>> Right, these are the kinds of questions such a comment ought to answer.
>>
>> Let's see if anyone chimes in here, otherwise let's complain in the
>> original thread, since it had nothing to do with this one.
>
> So nothing has happened. Let's get your changes about the removed
> "shared" argument committed, and we can figure out the "preallocated"
> thing separately.

Ok, pushed. I included the brief comment on "preallocated", but it would
be nice to improve it further, per the above discussion.

Thanks!

- Heikki