On Thu, Mar 27, 2025 at 2:46 PM Matheus Alcantara
<matheusssilv97@gmail.com> wrote:
>
> Just my 0.2 cents. I also like the first approach even though I prefer
> the v4 version, but anyway, thanks very much for reviewing and
> committing!
Thanks for the patch!
FWIW, I strongly disliked about v4 that two separate parts of the
callback were responsible for advancing current_blocknum, one to
advance it past blocks we chose to skip and the other to advance it to
the next block.
for (; p->current_blocknum < p->last_exclusive; p->current_blocknum++)
and
if (p->current_blocknum < p->last_exclusive)
return p->current_blocknum++;
I found that alone to be undesirable, but once you add in
SKIP_PAGES_NONE, I think it was very hard to understand.
Besides that, when we implemented streaming read sequential scan, we
ended up making dedicated callbacks for the parallel and non-parallel
cases (see heap_scan_stream_read_next_parallel and
heap_scan_stream_read_next_serial) because it performed better than a
single combined callback with a branch. I didn't validate that amcheck
got the same performance benefit from the dedicated callbacks, but I
don't see why it would be any different.
- Melanie
