public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: some more include removal from headers
Date: Fri, 13 Mar 2026 17:14:03 -0400
Message-ID: <qox6yqkety63epa4puqc7qm2f4dzs72s6ejtxbf277yamb43ne@rd2u267htgzm> (raw)
In-Reply-To: <[email protected]>
References: <j32gzsc32267k4phni2zf5yjikpyfjbqvup22bvloqga6svvkz@ao3c6ygpmz5p>
<[email protected]>
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
view thread (7+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: some more include removal from headers
In-Reply-To: <qox6yqkety63epa4puqc7qm2f4dzs72s6ejtxbf277yamb43ne@rd2u267htgzm>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox