Re: BitmapHeapScan streaming read user and prelim refactoring

On Thu, Feb 13, 2025 at 10:46 AM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>
> I reviewed v29 today, and I think it's pretty much ready to go.
>
> The one part where I don't quite get is 0001, which replaces a
> FLEXIBLE_ARRAY_MEMBER array with a fixed-length array. It's not wrong,
> but I don't quite see the benefits / clarity. And I think Thomas might
> be right we may want to make this dynamic, to save memory.
>
> Not a blocker, but I'd probably skip 0001 (unless it's required by the
> later parts, I haven't checked/tried).

So, on master, it already pallocs an array of size MAX_TUPLES_PER_PAGE
(which is hard-coded in the tidbitmap API to MaxHeapTuplesPerPage) --
see tbm_begin_private_iterate().

So we always palloc the same amount of memory. The reason I changed it
from a flexible sized array to a fixed size is that we weren't using
the flexibility and having a flexible sized array in the
TBMIterateResult meant it couldn't be nested in another struct. Since
I have to separate the TBMIterateResult and TBMIterator to implement
the read stream API for BHS, once I separate them, I nest the
TBMIterateResult in the GinScanEntry. If the array of offsets is
flexible sized, then I would have to manage that memory separately in
GIN code for the TBMIterateResult..

So, 0001 isn't a change in the amount of memory allocated.

With the read stream API, these TBMIterateResults are palloc'd just
like we palloc'd one in master. However, we have to have more than one
at a time.

- Melanie