On 21/01/2020 12:45, Kyotaro Horiguchi wrote:
> At Mon, 20 Jan 2020 17:24:07 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
>> Separating XLogBeginRead seems reasonable. The annoying recptr trick
>> is no longer needed. In particular some logical-decoding stuff become
>> far cleaner by the patch.
>>
>> fetching_ckpt of ReadRecord is a bit annoying but that coundn't be
>> moved outside to, say, ReadCheckpointRecord in a clean way. (Anyway
>> XLogBeginRead is not the place, of course).
>>
>> As I looked through it, it looks good to me but give me some more time
>> to look closer.
>
> It seems to me that it works perfectly, and everything looks good to
> me except one point.
>
> - * In this case, the passed-in record pointer should already be
> + * In this case, EndRecPtr record pointer should already be
>
> I'm not confident but isn't the "record pointer" redundant? EndRecPtr
> seems containing that meaning, and the other occurances of that kind
> of variable names are not accompanied by that.
I fixed that, fixed the XLogFindNextRecord() function so that it
positions the reader like XLogBeginRead() does (I had already adjusted
the comments to say that, but the function didn't actually do it), and
pushed. Thanks for the review!
I'll continue looking at the callback API changes on Monday.
- Heikki