Subject: Re: pgstat include expansion

From: Alvaro Herrera <[email-masked]>
Date: Mon, 16 Feb 2026 17:18:36 +0100

On 2026-Feb-13, Andres Freund wrote:

> commit 7e638d7f509
> Author: Álvaro Herrera <[email-masked]>
> 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.

> 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).

Yeah, I intended not to add access/transam.h here and instead
forward-declare FullTransactionId, but forgot to actually remove it
before pushing.  So +1 for what you're doing here with that.  Same with
relcache.h and most of the other changes you're doing here.

I don't think the removal of pg_list.h works terribly well though, as
that is something that I would expect to be pretty much everywhere, so
it seems somewhat pointless to try to avoid it in this file.

I'm not very sure what to do about LogicalRepWorkerType ... will
follow-up to Amit's reply.

I think we should in addition remove some includes that were kept during
recent IWYU hacking "for backwards compatibility", and instead include
them explicitly wherever they are needed.  The attached does that.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"You're _really_ hosed if the person doing the hiring doesn't understand
relational systems: you end up with a whole raft of programmers, none of
whom has had a Date with the clue stick."              (Andrew Sullivan)
https://postgr.es/m/[email-masked]

---

From: Andres Freund <[email-masked]>
Date: Mon, 16 Feb 2026 12:41:02 -0500

Hi,

> relcache.h and most of the other changes you're doing here.

Ah, great.

> it seems somewhat pointless to try to avoid it in this file.

Yea, I was on the fence about that one.

It does seem pretty crappy that pg_list.h is so widely included and in turn
includes nodes/nodes.h, which then depends on the generated nodetags.h.  For
one it forces an unnecessary rebuild of too much if you add a new node type,
but it also just aesthetically feels wrong.

But avoiding it just here does, as you say, not save very much.

> them explicitly wherever they are needed.  The attached does that.

FWIW, the "for backwards compatibility" includes are from prep work towards
shared memory stats. Before that all the stuff in backend_status.h,
backend_progress.h and wait_event.h was in pgstat.h, which was quite
unwieldy. The creation of those headers would have broken a lot of extensions
if we had not provided those backward compat includes from pgstat.h.

I suspect it would still break a fair number of extensions if removed those
includes today. Not sure if the improvement is worth it?  But even if we
decide not to remove the backward compat includes, I think we should fix our
own code to include the right headers directly, rather than rely on doing so
via pgstat.h.

FWIW, your patch that you sent subsequently doesn't seem to apply cleanly for
me? I think it's perhaps based on an older tree, it doesn't know about the
conflict.h include yet...  And it fails to build e.g. due to miscadmin.h
defines around BackendType not being indirectly included in pgstat.h anymore,
but there are many other failures...

Greetings,

Andres Freund