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

On 20.11.2018 6:30, Michael Paquier wrote:
> On Mon, Nov 19, 2018 at 10:48:06AM +0500, Andrey Lepikhov wrote:
>> According to my experience, I clarify some comments to avoid this mistakes
>> in the future (see attachment).
> 
> No objections from here.
> 
>> - * The returned pointer (or *errormsg) points to an internal buffer that's
>> - * valid until the next call to XLogReadRecord.
>> + * The returned pointer (or *errormsg) points to an internal read-only buffer
>> + * that's valid until the next call to XLogReadRecord.
> 
> Not sure that this bit adds much.
Ok
> 
>> -	/* Buffer for current ReadRecord result (expandable) */
>> +	/*
>> +	 * Buffer for current ReadRecord result (expandable).
>> +	 * Used in the case, than current ReadRecord result don't fit on the
>> +	 * currently read WAL page.
>> +	 */
> 
> Yeah, this one is not entirely true now.  What about the following
> instead:
> -   /* Buffer for current ReadRecord result (expandable) */
> +   /*
> +    * Buffer for current ReadRecord result (expandable), used when a record
> +    * crosses a page boundary.
> +    */

I agree


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