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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
>From 92d8e460c4f65f8fca735d90f02fb128f53abeef Mon Sep 17 00:00:00 2001 From: Andres Freund <[email protected]> Date: Fri, 13 Feb 2026 17:05:15 -0500 Subject: [PATCH v1] WIP: Reduce includes in pgstat.h --- src/include/pgstat.h | 13 +++++++------ src/include/replication/conflict.h | 5 +++-- src/backend/commands/functioncmds.c | 1 + src/backend/executor/execReplication.c | 1 + src/backend/replication/logical/conflict.c | 1 + src/backend/storage/ipc/procsignal.c | 1 + src/backend/utils/activity/pgstat_subscription.c | 4 ++-- .../test_custom_stats/test_custom_fixed_stats.c | 2 +- .../test_custom_stats/test_custom_var_stats.c | 1 + 9 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/include/pgstat.h b/src/include/pgstat.h index fff7ecc2533..a2021a0e66f 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -11,18 +11,20 @@ #ifndef PGSTAT_H #define PGSTAT_H -#include "access/transam.h" /* for FullTransactionId */ #include "datatype/timestamp.h" #include "portability/instr_time.h" #include "postmaster/pgarch.h" /* for MAX_XFN_CHARS */ -#include "replication/conflict.h" -#include "replication/worker_internal.h" +#include "replication/conflict.h" /* for CONFLICT_NUM_TYPES */ #include "utils/backend_progress.h" /* for backward compatibility */ /* IWYU pragma: export */ #include "utils/backend_status.h" /* for backward compatibility */ /* IWYU pragma: export */ #include "utils/pgstat_kind.h" -#include "utils/relcache.h" #include "utils/wait_event.h" /* for backward compatibility */ /* IWYU pragma: export */ +/* to avoid including access/transam.h, for FullTransactionId */ +typedef struct FullTransactionId FullTransactionId; + +/* to avoid including utils/relcache.h */ +typedef struct RelationData *Relation; /* ---------- * Paths for the statistics files (relative to installation's $PGDATA). @@ -775,8 +777,7 @@ extern PgStat_SLRUStats *pgstat_fetch_slru(void); * Functions in pgstat_subscription.c */ -extern void pgstat_report_subscription_error(Oid subid, - LogicalRepWorkerType wtype); +extern void pgstat_report_subscription_error(Oid subid, int wtype); extern void pgstat_report_subscription_conflict(Oid subid, ConflictType type); extern void pgstat_create_subscription(Oid subid); extern void pgstat_drop_subscription(Oid subid); diff --git a/src/include/replication/conflict.h b/src/include/replication/conflict.h index 1cade336c91..4e1a8a2303f 100644 --- a/src/include/replication/conflict.h +++ b/src/include/replication/conflict.h @@ -10,14 +10,15 @@ #define CONFLICT_H #include "access/xlogdefs.h" -#include "nodes/pg_list.h" -#include "utils/timestamp.h" +#include "datatype/timestamp.h" /* Avoid including execnodes.h here */ typedef struct EState EState; typedef struct ResultRelInfo ResultRelInfo; typedef struct TupleTableSlot TupleTableSlot; +/* To avoid including pg_list.h */ +typedef struct List List; /* * Conflict types that could occur while applying remote changes. diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index a516b037dea..242372b1e68 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -34,6 +34,7 @@ #include "access/htup_details.h" #include "access/table.h" +#include "access/xact.h" #include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/indexing.h" diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 743b1ee2b28..c1007c1b41b 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -27,6 +27,7 @@ #include "commands/trigger.h" #include "executor/executor.h" #include "executor/nodeModifyTable.h" +#include "nodes/pg_list.h" #include "replication/conflict.h" #include "replication/logicalrelation.h" #include "storage/lmgr.h" diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index ca71a81c7bf..73cca04ffa5 100644 --- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -17,6 +17,7 @@ #include "access/commit_ts.h" #include "access/tableam.h" #include "executor/executor.h" +#include "nodes/pg_list.h" #include "pgstat.h" #include "replication/conflict.h" #include "replication/worker_internal.h" diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index 5d33559926a..4e32a29766c 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -23,6 +23,7 @@ #include "pgstat.h" #include "port/pg_bitutils.h" #include "replication/logicalworker.h" +#include "replication/logicalctl.h" #include "replication/walsender.h" #include "storage/condition_variable.h" #include "storage/ipc.h" diff --git a/src/backend/utils/activity/pgstat_subscription.c b/src/backend/utils/activity/pgstat_subscription.c index 500b1899188..adeff040075 100644 --- a/src/backend/utils/activity/pgstat_subscription.c +++ b/src/backend/utils/activity/pgstat_subscription.c @@ -25,7 +25,7 @@ * Report a subscription error. */ void -pgstat_report_subscription_error(Oid subid, LogicalRepWorkerType wtype) +pgstat_report_subscription_error(Oid subid, int wtype) { PgStat_EntryRef *entry_ref; PgStat_BackendSubEntry *pending; @@ -34,7 +34,7 @@ pgstat_report_subscription_error(Oid subid, LogicalRepWorkerType wtype) InvalidOid, subid, NULL); pending = entry_ref->pending; - switch (wtype) + switch ((LogicalRepWorkerType) wtype) { case WORKERTYPE_APPLY: pending->apply_error_count++; diff --git a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c index 908bd18a7c7..fb74f3dc5ad 100644 --- a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c @@ -16,8 +16,8 @@ #include "access/htup_details.h" #include "funcapi.h" #include "pgstat.h" -#include "utils/builtins.h" #include "utils/pgstat_internal.h" +#include "utils/timestamp.h" PG_MODULE_MAGIC_EXT( .name = "test_custom_fixed_stats", diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c index 64a8fe63cce..da28afbd929 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c @@ -12,6 +12,7 @@ */ #include "postgres.h" +#include "access/htup_details.h" #include "common/hashfn.h" #include "funcapi.h" #include "storage/dsm_registry.h" -- 2.46.0.519.g2e7b89e038
