Hi, On 2026-03-13 20:32:53 +0100, Álvaro Herrera wrote: > On 2026-Mar-13, Andres Freund wrote: > > > Kinda wonder if funcapi.h should export tuplestore.h. That'd avoid needing > > adding the dedicated tuplestore.h in so many places, and as the use of > > tuplestore is basically required for funcapi.h users, it seems like it'd be > > fine semantically? > > Hmm. I think there are plenty of SRF functions that use the > value-per-call mechanics instead of a tuplestore (which don't need > tuplestore.h), but I wouldn't be opposed to doing that. I was going to > complain about that change dragging tuptable.h into funcapi.h -- until I > realized that funcapi.h already includes tuptable.h directly itself. So > I don't see any downsides.
Cool. > > If we could go back in time I'd wrap the tuplestore_puttuple() for the SRF > > use > > case into an SRF specific wrapper, but my time travel capabilities have not > > developed, despite no lack of trying. > > hah! (We could still change this now, and while it wouldn't change > older code or existing third party extensions, it might definitely make > things better going forward; the future being longer than the past, this > might be good anyhow. But that's a matter for a separate thread.) Agreed... > > The need for dsa.h and condition_variable.h just is from > > ParallelBitmapHeapState - which isn't actually an executor node and never > > needed outside of nodeBitmapHeapscan.c - so it seems better to move it > > there? > > > > Added a commit for that. > > Whoa, nice! Thanks. > > Your patch numbering says 5/6, but there's only 5 attached, I assume that > > was > > intentional? > > Yeah, the last one was about removing tidbitmap.h from genam.h -- I left > it out at the last minute because it's unrelated to execnodes.h. > Attached here. Nice improvement. LGTM. > > I couldn't help myself to slim down execnodes.h further. Not sure if all of > > them are quite worth it. > > The relpath.h one is definitely a good win. Yea. > Not sure about stringinfo.h, which doesn't change very often and doesn't > include anything else. It seemed worth it because it's not used by plannodes.h, it's a leftover from before a05dc4d7fd5. > I'm doubtful about the bitmapset.h removal gaining > us much either (because the really nasty one below, nodetags.h, is > unavoidable anyway.) Yea, I was very much on the fence with that one. > > With all the commits combined very little low-level stuff is still > > included. The worst is probably instr_time.h. > > Hm, that one is worth thinking about. But with only this much, it's > already a huge win. Agreed, we can tackle that one separately. There's multiple paths for it too, e.g. a separate header for the instr_time type or moving towards not not needing to include instrument[_node].h. How would you like to work towards committing these? I'm am more than happy for you to commit everything, but I could also help. Greetings, Andres Freund
