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

Hello.

At Tue, 23 Oct 2018 10:25:27 +0500, Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru> wrote in <2553f2b0-0e39-eb0e-d382-6c0ed08ca8ae(at)postgrespro(dot)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?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center