Re: Use streaming read API in ANALYZE

On Thu, Apr 04, 2024 at 02:03:30PM +0300, Nazir Bilal Yavuz wrote:
> 
> On Wed, 3 Apr 2024 at 23:44, Melanie Plageman <melanieplageman@gmail.com> wrote:
> >
> > I don't see the point of BlockSampler_HasMore() anymore. I removed it in
> > the attached and made BlockSampler_Next() return InvalidBlockNumber
> > under the same conditions. Is there a reason not to do this? There
> > aren't other callers. If the BlockSampler_Next() wasn't part of an API,
> > we could just make it the streaming read callback, but that might be
> > weird as it is now.
> 
> I agree. There is no reason to have BlockSampler_HasMore() after
> streaming read API changes.
> 
> > That and my other ideas in attached. Let me know what you think.
> 
> I agree with your changes but I am not sure if others agree with all
> the changes you have proposed. So, I didn't merge 0001 and your ideas
> yet, instead I wrote a commit message, added some comments, changed ->
> 'if (bs->t >= bs->N || bs->m >= bs->n)' to 'if (K <= 0 || k <= 0)' and
> attached it as 0002.

I couldn't quite let go of those changes to acquire_sample_rows(), so
attached v9 0001 implements them as a preliminary patch before your
analyze streaming read user. I inlined heapam_scan_analyze_next_block()
entirely and made heapam_scan_analyze_next_tuple() a static function in
commands/analyze.c (and tweaked the name).

I made a few tweaks to your patch since it is on top of those changes
instead of preceding them. Then 0003 is removing BlockSampler_HasMore()
since it doesn't make sense to remove it before the streaming read user
was added.

- Melanie