Re: BitmapHeapScan streaming read user and prelim refactoring - Mailing list pgsql-hackers

On Tue, May 14, 2024 at 2:18 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, May 13, 2024 at 10:05:03AM -0400, Melanie Plageman wrote:
> > Remove the assert and reset the field on which it previously asserted to
> > avoid incorrectly emitting NULL-filled tuples from a previous scan on
> > rescan.
>
> > -     Assert(scan->rs_empty_tuples_pending == 0);
> > +     scan->rs_empty_tuples_pending = 0;
>
> Perhaps this should document the reason why the reset is done in these
> two paths rather than let the reader guess it?  And this is about
> avoiding emitting some tuples from a previous scan.

I've added a comment to heap_rescan() in the attached v5. Doing so
made me realize that we shouldn't bother resetting it in
heap_endscan(). Doing so is perhaps more confusing, because it implies
that field may somehow be used later. I've removed the reset of
rs_empty_tuples_pending from heap_endscan().

> > +SET enable_indexonlyscan = off;
> > +set enable_indexscan = off;
> > +SET enable_seqscan = off;
>
> Nit: adjusting the casing of the second SET here.

I've fixed this. I've also set enable_material off as I mentioned I
might in my earlier mail.

- Melanie