Hi,
A few recent-ish changes have substantially expanded the set of headers
included in pgstat.h. As pgstat.h is pretty widely included, that strikes me
as bad.
commit f6a4c498dcf
Author: Amit Kapila <akapila@postgresql.org>
Date: 2025-11-07 08:05:08 +0000
Add seq_sync_error_count to subscription statistics.
added an include of replication/worker_internal.h, which in turn includes a
lot of other headers. It's also seems somewhat obvious that a header named
_internal.h shouldn't be included in something as widely included as pgstat.h
commit 7e638d7f509
Author: Álvaro Herrera <alvherre@kurilemu.de>
Date: 2025-09-25 14:45:08 +0200
Don't include execnodes.h in replication/conflict.h
added an include of access/transam.h, which I don't love being included,
although it's less bad than some of the other cases. It's not actually to
blame for that though, as it shrank what was included noticeably.
commit 6c2b5edecc0
Author: Amit Kapila <akapila@postgresql.org>
Date: 2024-09-04 08:55:21 +0530
Collect statistics about conflicts in logical replication.
added an include of conflict.h, which in turn includes utils/timestamp.h,
which includes fmgr.h (and a lot more, before 7e638d7f509).
I think now that we rely on C11, we actually could also forward-declare enum
typedefs. That would allow us to avoid including
worker_internal.h. Unfortunately I think C++ might throw a wrench in the mix
for that - IIUC it only allows forward declaring enums when using 'enum
class', but I don't think we can rely on that. I think the best solution
would be to move the typedef to a more suitable header, but baring that, I
guess just passing an int would do.
By forward declaring FullTransactionId, we can avoid including
access/transam.h
conflict.h includes utils/timestamp.h, even though it only needs the
types. Using datatype/timestamp.h suffices (although it requires some fixups
elsewhere).
conflict.h includes nodes/pg_list.h, which does in turn include nodes/nodes.h,
which, while not the worst, seems unnecessary. Another forward declaration
solves that.
The attached patch is just a prototype.
I also don't like that pgstat.h includes backend_status.h, which includes
things like pqcomm.h and miscadmin.h. But that's - leaving some moving around
aside - of a lot longer standing. We probably should move BackendType out of
miscadmin.h one of these days. Not sure what to do about the pqcomm.h
include...
Greetings,
Andres Freund
