On Sun, Sep 11, 2016 at 3:13 PM, Peter Geoghegan <p...@heroku.com> wrote:
> * Doesn't this code need to call MemoryContextAllocHuge() rather than 
> palloc()?:
>> @@ -709,18 +765,19 @@ LogicalTapeRewind(LogicalTapeSet *lts, int tapenum, 
>> bool forWrite)
>>             Assert(lt->frozen);
>>             datablocknum = ltsRewindFrozenIndirectBlock(lts, lt->indirect);
>>         }
>> +
>> +       /* Allocate a read buffer */
>> +       if (lt->buffer)
>> +           pfree(lt->buffer);
>> +       lt->buffer = palloc(lt->read_buffer_size);
>> +       lt->buffer_size = lt->read_buffer_size;

Of course, when you do that you're going to have to make the new
"buffer_size" and "read_buffer_size" fields of type "Size" (or,
possibly, "int64", to match tuplesort.c's own buffer sizing variables
ever since Noah added MaxAllocSizeHuge). Ditto for the existing "pos"
and "nbytes" fields next to "buffer_size" within the struct
LogicalTape, I think. ISTM that logtape.c blocknums can remain of type
"long", though, since that reflects an existing hardly-relevant
limitation that you're not making any worse.

More generally, I think you also need to explain in comments why there
is a "read_buffer_size" field in addition to the "buffer_size" field.
Peter Geoghegan

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to