Re: some more include removal from headers

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