Re: pgstat include expansion
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Alvaro Herrera <alvherre@kurilemu.de>
Cc: pgsql-hackers@postgresql.org, Amit Kapila <amit.kapila16@gmail.com>, Michael Paquier <michael@paquier.xyz>
Date: 2026-02-16T17:41:02Z
Lists: pgsql-hackers
Hi, On 2026-02-16 17:18:36 +0100, Alvaro Herrera wrote: > 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. Ah, great. > 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. 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. > 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. 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
Commits
-
Don't include wait_event.h in pgstat.h
- 868825aaeb40 19 (unreleased) landed
-
Don't include proc.h in shm_mq.h
- a2c89835f512 19 (unreleased) landed
-
Don't include latch.h in libpq/libpq.h
- 7b9b620d8fc7 19 (unreleased) landed
-
Reduce includes in pgstat.h
- 7bb50dd7d6b9 19 (unreleased) landed
-
Avoid including utils/timestamp.h in conflict.h.
- 308622edf174 19 (unreleased) landed
-
Avoid including worker_internal.h in pgstat.h.
- 9842e8aca096 19 (unreleased) landed
-
Add seq_sync_error_count to subscription statistics.
- f6a4c498dcf6 19 (unreleased) cited
-
Don't include execnodes.h in replication/conflict.h
- 7e638d7f5093 19 (unreleased) cited
-
Collect statistics about conflicts in logical replication.
- 6c2b5edecc0d 18.0 cited