Hi,
On Sat, Jun 8, 2024 at 5:24 PM Nitin Jadhav <
nitinjadhavpostgres@gmail.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
