Re: [PATCH] XLogReadRecord returns pointer to currently read page

On 22.10.2018 2:06, Heikki Linnakangas wrote:
> On 17/08/2018 06:47, Andrey Lepikhov wrote:
>> I propose the patch for fix one small code defect.
>> The XLogReadRecord() function reads the pages of a WAL segment that
>> contain a WAL-record. Then it creates a readRecordBuf buffer in private
>> memory of a backend and copy record from the pages to the readRecordBuf
>> buffer. Pointer 'record' is set to the beginning of the readRecordBuf
>> buffer.
>>
>> But if the WAL-record is fully placed on one page, the XLogReadRecord()
>> function forgets to bind the "record" pointer with the beginning of the
>> readRecordBuf buffer. In this case, XLogReadRecord() returns a pointer
>> to an internal xlog page. This patch fixes the defect.
> 
> Hmm. I agree it looks weird the way it is. But I wonder, why do we even 
> copy the record to readRecordBuf, if it fits on the WAL page? Returning 
> a pointer to the xlog page buffer seems OK in that case. What if we 
> remove the memcpy(), instead?

It depends on the PostgreSQL roadmap. If we want to compress main data 
and encode something to reduce a WAL-record size, than the 
representation of the WAL-record in shared buffers (packed) and in local 
memory (unpacked) will be different and the patch is needed.
If something like this should not be in the PostgreSQL core, then it is 
more efficient to remove memcpy(), of course.
I vote for the patch.

-- 
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company