Date: Mon, 23 Mar 2026 08:18:05 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: =?iso-8859-1?Q?=C1lvaro?= Herrera <alvherre@kurilemu.de>
Cc: Michael Paquier <michael@paquier.xyz>,
	Andres Freund <andres@anarazel.de>, Jeff Davis <pgsql@j-davis.com>,
	Greg Sabino Mullane <htamfids@gmail.com>,
	"L. pgsql-hackers" <pgsql-hackers@lists.postgresql.org>
Subject: Re: Adding locks statistics
Message-ID: <acD3PUbZ0ZER+hCl@ip-10-97-1-34.eu-west-3.compute.internal>
References: <e38089eb-3619-4434-b888-397175c4eaa9@app.fastmail.com>
 <202603221856.iwlhitt6dxxx@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202603221856.iwlhitt6dxxx@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/acD3PUbZ0ZER%2BhCl%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

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