Re: Use WALReadFromBuffers in more places

Hi,

On Sat, Jun 8, 2024 at 5:24 PM Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
wrote:
>
> I spent some time examining the patch. Here are my observations from the
review.

Thanks.

> - I believe there’s no need for an extra variable ‘nbytes’ in this
> context. We can repurpose the ‘count’ variable for the same function.
> If necessary, we might think about renaming ‘count’ to ‘nbytes’.

'count' variable can't be altered once determined as the page_read
callbacks need to return the total number of bytes read. However, I ended
up removing 'nbytes' like in the attached v2 patch.

> - The operations performed by logical_read_xlog_page() and
> read_local_xlog_page_guts() are identical. It might be beneficial to
> create a shared function to minimize code repetition.

IMO, creating another function to just wrap two other functions doesn't
seem good to me.

I attached v2 patches for further review. No changes in 0002 patch.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com