Re: Use WALReadFromBuffers in more places

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: Jingtang Zhang <mrdrivingduck@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-06-12T16:40:03Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Read WAL directly from WAL buffers.

Attachments

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