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

On 26.10.2018 10:33, Kyotaro HORIGUCHI wrote:
> Hello.
> 
> At Tue, 23 Oct 2018 10:25:27 +0500, Andrey Lepikhov <a.lepikhov@postgrespro.ru> wrote in <2553f2b0-0e39-eb0e-d382-6c0ed08ca8ae@postgrespro.ru>
>>
>> On 23.10.2018 0:53, Heikki Linnakangas wrote:
>>> I'd expect the decompression to read from the on-disk buffer, and
>>> unpack to readRecordBuf, I still don't see a need to copy the packed
>>> record to readRecordBuf. If there is a need for that, though, the
>>> patch that implements the packing or compression can add the memcpy()
>>> where it needs it.
>>
>> I agree with it. Eventually, placement of the WAL-record can be
>> defined by comparison the record, readBuf and readRecordBuf pointers.
>> In attachment new version of the patch.
> 
> This looks quite clear and what it does is reasonable to me.
> Applies cleanly on top of current master and no regression seen.
> 
> 
> Just one comment. This patch leaves the following code.
> 
>   >		/* Record does not cross a page boundary */
>   >		if (!ValidXLogRecord(state, record, RecPtr))
>   >			goto err;
>   >
>   >		state->EndRecPtr = RecPtr + MAXALIGN(total_len);
> !>
>   >		state->ReadRecPtr = RecPtr;
>   >	}
> 
> The empty line (marked by '!') looks a bit too much standing out
> after this patch. Could you remove the line? Then could you
> transpose the two assignments if you don't mind?

Thanks, see attachment.

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