Re: Table AM Interface Enhancements

On Mon, 15 Apr 2024 at 22:09, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Apr 15, 2024 at 12:37 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>
> wrote:
> > In my understanding, the downside of 041b96802ef is bringing
> read_stream* things from being heap-only-related up to the level of
> acquire_sample_rows() that is not supposed to be tied to heap. And changing
> *_analyze_next_block() function signature to use ReadStream explicitly in
> the signature.
>
> I don't think that really clarifies anything. The ReadStream is
> basically just acting as a wrapper for a stream of block numbers, and
> the API took a BlockNumber before. So why does it make any difference?
>
> If I understand correctly, Alexander thinks that, before 041b96802ef,
> the block number didn't necessarily have to be the physical block
> number on disk, but could instead be any 32-bit quantity that the
> table AM wanted to pack into the block number. But I don't think
> that's true, because acquire_sample_rows() was already passing those
> block numbers to PrefetchBuffer(), which already requires physical
> block numbers.
>

Hi, Robert!

Why it makes a difference looks a little bit unclear to me, I can't comment
on this. I noticed that before 041b96802ef we had a block number and block
sampler state that tied acquire_sample_rows() to the actual block
structure. After we have the whole struct ReadStream which doesn't comprise
just a wrapper for the same variables, but the state that ties
acquire_sample_rows() to the streaming read algorithm (and heap). Yes, we
don't have other access methods other than heap implemented for analyze
routine, so the patch works anyway, but from the view on
acquire_sample_rows() as a general method that is intended to have
different implementations in the future it doesn't look good.

It's my impression on 041b96802ef, please forgive me if I haven't
understood something.

Regards,
Pavel Borisov
Supabase