Re: pgstattuple: Use streaming read API in pgstatindex functions - Mailing list pgsql-hackers

Hi Kato-san,

Thanks for looking into this.

On Thu, Oct 16, 2025 at 4:21 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
>
> Hi,
>
> On Wed, Oct 15, 2025 at 10:25 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>>
>> Hi,
>>
>> Here’s the updated summary report(cold cache, fragmented index), now including results for the streaming I/O +
io_uringconfiguration. 
>>
>
> Thank you for the additional tests. I can see the image on Gmail, but I cannot on pgsql-hackers archive [0], so it
mightbe a good idea to attach it and not to paste it on the body. 

Please see the attachment.

>
>
> I saw the patch and have a few minor comments.
>
> + p.current_blocknum = 1;
>
> To improve readability, how about using the following, which is consistent with nbtree.c [1]?
> p.current_blocknum = BTREE_METAPAGE + 1;
>
> Similarly, for hash index:
> p.current_blocknum = HASH_METAPAGE + 1;

This is more readable. I made the replacements.

>
> + /* Unlock and release buffer */
>   UnlockReleaseBuffer(buf);
>
> I think this comment is redundant since the function name UnlockReleaseBuffer is self-explanatory. I suggest omitting
itfrom pgstathashindex and removing the existing one from pgstatindex_impl. 

UnlockReleaseBuffer seems clearer and simpler than the original

> LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
> ReleaseBuffer(buffer);

So the comment might be less meaningful for UnlockReleaseBuffer. I
removed it as you suggested.

Best,
Xuneng