Hi, On Sun, Mar 22, 2026 at 08:37:47PM +0100, Álvaro Herrera wrote: > On 2026-Mar-21, Álvaro Herrera wrote: > > > I checked this, and found a couple of headers that can benefit from a > > removal, as shown in the attached patches. > > I looked again and found some more; the first 14 patches attached here > do so. A couple of them have no fallout to speak of, which I find > really surprising; for the majority we just need a couple of extra > includes somewhere or a typedef or two. I unleashed CI on it to see > what would happen, > https://cirrus-ci.com/build/5522804717649920
Thanks for looking at it! I looked at the C files changes that now need to include lock.h (or other ones) directly: verify_heapam.c: lock.h was indirectly included through procarray.h heapam_handler.c: lock.h was indirectly included through heapam.h -> tableam.h -> vacuum.h relation.c: lock.h was indirectly included through namespace.h reloptions.c: lock.h was indirectly included through reloptions.h indexam.c: lock.h was indirectly included through reloptions.h relcache.c: lock.h was indirectly included through reloptions.h syscache.c: lock.h was indirectly included through lmgr.h discard.c: lock.h was indirectly included through namespace.h pg_subscription.c: lock.h was indirectly included through heapam.h -> tableam.h -> vacuum.h nbtree.c: lock.h was indirectly included through vacuum.h nbtutils.c: lock.h was indirectly included through reloptions.h pg_inherits.c: lock.h was indirectly included through pg_inherits.h inherit.c: lock.h was indirectly included through pg_inherits.h conversioncmds.c: lock.h was indirectly included parse_func.h -> namespace.h tablespace.c: lock.h was indirectly included through heapam.h -> tableam.h -> vacuum.h parse_oper.c: lock.h was indirectly included through parse_func.h -> namespace.h sequencesync.c: lock.h was indirectly included through worker_internal.h ts_cache.c: lock.h was indirectly included through namespace.h So the changes done in your patches make sense to me. I have 2 comments: 1/ wait_event.c -#include "storage/lmgr.h" /* for GetLockNameFromTagType */ -#include "storage/lwlock.h" /* for GetLWLockIdentifier */ +#include "storage/lmgr.h" +#include "storage/lwlock.h" +#include "storage/shmem.h" #include "storage/spin.h" +#include "utils/hsearch.h" hsearch.h is already included into shmem.h so its direct include is not needed. That said wait_event.c needs it so including it directly might make sense just from a coding "style" point of view (given that it is harmless as it is protected by ifndef HSEARCH_H). 2/ Not directly linked to your patches It looks like that aio_funcs.c does not need lock.h (reported by include-what-you-use). If we remove its direct include, it's still indirectly included through proc.h though. But I think that removing its direct include makes sense as it's not needed at all. If we still need to discuss about those lock.h related changes, maybe it's worth creating a dedicated thread? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
