On Wed, 2 Apr 2025 at 19:48, Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2025-04-02 18:58:11 +0200, Matthias van de Meent wrote: > > And here it is, v6 of the patchset, rebased up to master @ 764d501d. > > Thanks! > > Does anybody have an opinion about how non-invasive to be in the > back-branches? The minimal version is something like this diff: [...] > But it seems a bit weird to continue checking SO_NEED_TUPLES (which is what > need_tuples ends up as) in other parts of the code. But it's less work to > backpatch that way. Obviously we can't remove the relevant struct fields in > the backbranches.
A minimal version seems fine to me. > Other notes: > > - Should we commit the test showing that the naive implementation of > index-only-bitmap-heapscan is broken, in case somebody wants to re-introduce > it? I'd prefer that, yes. > If so, I think we should not backpatch the test. If it turns out to not be > stable, it's a pain to fix test stability issues over multiple branches. That's fair. But if the reason for not adding a test is potential instability we could just as well add it now and remove it if it actually proves to be unstable. > - We have some superfluous includes in nodeBitmapHeapscan.c - but I think > that's not actually the fault of this patch. Seems the read-stream patch > should have removed the at least the includes of visibilitymap.h, bufmgr.h > and spccache.h? And b09ff53667f math.h... I have no strong opinion about this. Thank you for picking this up! Kind regards, Matthias van de Meent Neon (https://neon.tech)
