From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
---|---|
To: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Subject: | Re: pgstattuple: Use streaming read API in pgstatindex functions |
Date: | 2025-10-13 08:42:04 |
Message-ID: | CABPTF7VWdwJ52cUhmt_7vsEomeRcP9k8JaMaPHvE-O7KM9KwMA@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Bilal,
Thanks for looking into this.
On Mon, Oct 13, 2025 at 3:00 PM Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:
>
> Hi,
>
> Thank you for working on this!
>
> On Mon, 13 Oct 2025 at 06:20, Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> >
> > Fix indentation issue in v1.
>
> I did not look at the benchmarks, so here are my code comments.
>
> - I would avoid creating a new scope for the streaming read. While it
> makes the streaming code easier to interpret, it introduces a large
> diff due to indentation changes.
>
> - I suggest adding the following comment about streaming batching, as
> it is used in other places:
>
> /*
> * It is safe to use batchmode as block_range_read_stream_cb takes no
> * locks.
> */
>
> - For the '/* Scan all blocks except the metapage using streaming
> reads */' comments, it might be helpful to clarify that the 0th page
> is the metapage. Something like: '/* Scan all blocks except the
> metapage (0th page) using streaming reads */'.
>
> Other than these comments, the code looks good to me.
>
Here is patch v3. The comments have been added, and the extra scope
({}) has been removed as suggested.
Best,
Xuneng
Attachment | Content-Type | Size |
---|---|---|
v3-0001-pgstattuple-Use-streaming-read-API-in-pgstatindex.patch | application/octet-stream | 4.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | wenhui qiu | 2025-10-13 09:41:09 | Re: pgstattuple: Use streaming read API in pgstatindex functions |
Previous Message | David Rowley | 2025-10-13 08:31:35 | Re: Improve docs for n_distinct_inherited |