Feedback-ID: id4a34324:Fastmail
Date: Thu, 26 Feb 2026 11:27:04 -0500
From: Andres Freund <andres@anarazel.de>
To: Alvaro Herrera <alvherre@kurilemu.de>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@postgresql.org, 
	Amit Kapila <amit.kapila16@gmail.com>, Michael Paquier <michael@paquier.xyz>
Subject: Re: pgstat include expansion
Message-ID: <pap7mzhcxvuwlfdebjkh646ntyk4brtwm4dbocfpllwdccta5t@w3d7wz6mjpwv>
References: <55orfweo3sys7g5vstb3ufv7es2fxyi2tj7rrfwn4okasay5hw@eiktf4jpwnnz>
 <202602261523.yu3fs2tpzszl@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202602261523.yu3fs2tpzszl@alvherre.pgsql>
Archived-At: <https://www.postgresql.org/message-id/pap7mzhcxvuwlfdebjkh646ntyk4brtwm4dbocfpllwdccta5t%40w3d7wz6mjpwv>
Precedence: bulk

Hi,

On 2026-02-26 16:59:12 +0100, Alvaro Herrera wrote:
> On 2026-Feb-26, Andres Freund wrote:
> > On 2026-02-26 14:52:10 +0100, Alvaro Herrera wrote:
> > > This one removes wait_event.h from pgstat.h.  The only difference from
> > > the previous one, aside from the fact that I had to add pgstat.h to a
> > > few more .c files, is that xlogreader.c needs to include it inside the
> > > #ifndef FRONTEND; otherwise, "ninja -t missingdeps" complain about the
> > > file being used in pg_rewind and pg_waldump (seen in CI).
> >
> > I'm somewhat worried about the amount of breakage this will cause in
> > extensions.  Many extensions use something like
> >    WaitLatch(wait_event_info = WAIT_EVENT_EXTENSION)
> > and many of them won't have included wait_event.h.
>
> Yeah, I considered that, and instead of breaking these, to include
> wait_event.h in latch.h.  This sounds a very reasonable thing to do.
> But then I searched for other headers that have a wait_event_info
> parameter in one of their functions, and if we force those to also
> acquire wait_event.h, then the pollution is much worse than if we just
> leave it in pgstat.h.  (condition_variable.h, fd.h, proc.h, barrier.h).
>
> The problem is that some of those headers are in turn being included by
> others; most notably condition_variable.h is used in execnodes.h and
> others, and that makes a huge mess.  (And it can't be removed.)
>
> However, maybe we can just not be so principled about it, add it to
> latch.h, and then we don't break the whole world.  I'll try that.

FWIW, regardless of what we choose here (i.e. whether and where to
"unnecessarily" include wait_event.h), I'd be in favor of fixing all the
fallout in our source tree, just for cleanliness' sake. It's also presumably
the part of the patch that would be the most pain to keep updated.


I'm not sure that including in latch.h really would an improvement though -
that's included in proc.h and libpq.h, which both are fairly widely included
[1].


FWIW, I'm a lot less concerned with extension users of condition_variable.h,
fd.h, proc.h, barrier.h breaking due to removal of an implicit include, I
don't think they are remotely as widely used.


Greetings,

Andres Freund

[1] Although at least the include in libpq.h seems like it could trivially be
replaced by a forward declaration. There's only three .c files that need to be
fixed for that...