Re: BitmapHeapScan streaming read user and prelim refactoring

On Sun, Apr 7, 2024 at 7:38 AM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> On 4/7/24 06:17, Melanie Plageman wrote:
> >> What bothers me on 0006-0008 is that the justification in the commit
> >> messages is "future commit will do something". I think it's fine to have
> >> a separate "prepareatory" patches (I really like how you structured the
> >> patches this way), but it'd be good to have them right before that
> >> "future" commit - I'd like not to have one in v17 and then the "future
> >> commit" in v18, because that's annoying complication for backpatching,
> >> (and probably also when implementing the AM?) etc.
> >
> > Yes, I was thinking about this also.
> >
>
> Good we're on the same page.

Having thought about this some more I think we need to stop here for
17. v20-0001 and v20-0002 both make changes to the table AM API that
seem bizarre and unjustifiable without the other changes. Like, here
we changed all your parameters because someday we are going to do
something! You're welcome!

Also, the iterators in the TableScanDescData might be something I
could live with in the source code for a couple months before we make
the rest of the changes in July+. But, adding them does push the
TableScanDescData->rs_parallel member into the second cacheline, which
will be true in versions of Postgres people are using for years. I
didn't perf test, but seems bad.

So, yes, unfortunately, I think we should pick up on the BHS saga in a
few months. Or, actually, we should start focusing on that parallel
BHS + 0 readahead bug and whether or not we are going to fix it.

Sorry for the about-face.

- Melanie