Re: pgstat include expansion

On Mon, Feb 16, 2026 at 4:49 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> > On Feb 14, 2026, at 06:14, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > 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(at)postgresql(dot)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
> >
> >
>
> ```
> -pgstat_report_subscription_error(Oid subid, LogicalRepWorkerType wtype)
> +pgstat_report_subscription_error(Oid subid, int wtype)
> ```
>
> I am not a fan of changing LogicalRepWorkerType to int that feels like an unnecessary trade-off. LogicalRepWorkerType is defined in worker_internal.h, but as it’s used in pg_stat, it isn't “internal” anymore. So, maybe we should move the enum to a new worker_types.h and include that instead?
>

How about moving LogicalRepWorkerType to logicalworker.h as in the
attached and then include that in pgstat.h? This requires few other
adjustments as previously some of the includes were working indirectly
via worker_internal.h.

Fixed few warnings via different includes after the above change:
*
procsignal.c: In function ‘ProcessProcSignalBarrier’:
procsignal.c:580:61: warning: implicit declaration of function
‘ProcessBarrierUpdateXLogLogicalInfo’
[-Wimplicit-function-declaration]
580 | processed =
ProcessBarrierUpdateXLogLogicalInfo();
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This warning appears because ProcessBarrierUpdateXLogLogicalInfo
declaration present in logicalctl.h was included in procsignal.c
through pgstat.h-
>worker_internal->walreceiver.h->xlog.h->logicalctl.h. Since the proposed patch removes worker_internal.h from pgstat.h this warning appears. For this, we need to include logicalctl.h in procsignal.c.

*
functioncmds.c: In function ‘compute_return_type’:
functioncmds.c:157:17: warning: implicit declaration of function
‘CommandCounterIncrement’ [-Wimplicit-function-declaration]
157 | CommandCounterIncrement();

This warning appears because CommandCounterIncrement declaration
present in xact.h was getting included in functioncmds.c through
pgstat.h->worker_internal.h->logicalrelation.h->logicalproto.h->xact.h.
to fix this, we need to include "access/xact.h" in functioncmds.c.

*
On Windows:
[68/196] Compiling C object
src/test/modules/test_custom_stats/test_custom_var_stats.dll.p/test_custom_var_stats.c.obj
../src/test/modules/test_custom_stats/test_custom_var_stats.c(685):
warning C4047: '=': 'HeapTuple' differs in levels of indirection from
'int'

Previously HeapTuple seems to be detected via
pgstat_internal.h->pgstat.h->worker_internal.h->logicalrelation.h->index.h->execnodes.h->tupconvert.h->htup.h.
To fix, we need to include htup_details.h in test_custom_var_stats.c
similar to test_custom_fixed_stats.c.

--
With Regards,
Amit Kapila.