Feedback-ID: ie3de48e3:Fastmail
Date: Sat, 20 Sep 2025 15:13:24 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter@eisentraut.org>,
	pgsql-hackers <pgsql-hackers@postgresql.org>,
 Amit Kapila <amit.kapila16@gmail.com>
Subject: Re: allow benign typedef redefinitions (C11)
Message-ID: <202509201308.2skq4k5wkzgu@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="mrptdtvcz3f3uhfa"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2198104.1758331044@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/202509201308.2skq4k5wkzgu%40alvherre.pgsql>
Precedence: bulk


--mrptdtvcz3f3uhfa
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2025-Sep-19, Tom Lane wrote:

> Sooner or later we need to work out a subsystem hierarchy that
> can define which headers should be allowed to include which
> other ones.  I have no clear ideas about what that should look
> like, but I think the need for it gets ever more urgent.

I agree, we should do that.  In the meantime, it would help if people
were a tiny bit more careful with what headers they add to other
headers.

Here's a proposed fix.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.

--mrptdtvcz3f3uhfa
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Don-t-include-execnodes.h-in-replication-conflict.h.patch"

From 6a5170aea0f0c0e783b681537b077a2d7de51537 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Sat, 20 Sep 2025 15:07:58 +0200
Subject: [PATCH] Don't include execnodes.h in replication/conflict.h

... which silently propagates a lot of headers into many places
via pgstat.h, as evidenced by the variety of headers that this patch
needs to add to seemingly random places.  Add a minimum of typedefs to
conflict.h to be able to remove execnodes.h, and fix the fallout.

Discussion: https://postgr.es/m/202509191927.uj2ijwmho7nv@alvherre.pgsql
---
 src/backend/access/transam/multixact.c      | 1 +
 src/backend/access/transam/xlogrecovery.c   | 1 +
 src/backend/storage/ipc/waiteventset.c      | 1 +
 src/backend/utils/activity/pgstat_backend.c | 1 +
 src/include/pgstat.h                        | 1 +
 src/include/replication/conflict.h          | 9 ++++++++-
 6 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 8bf59d369f8..9d5f130af7e 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -79,6 +79,7 @@
 #include "pg_trace.h"
 #include "pgstat.h"
 #include "postmaster/autovacuum.h"
+#include "storage/condition_variable.h"
 #include "storage/pmsignal.h"
 #include "storage/proc.h"
 #include "storage/procarray.h"
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 346319338a0..52ff4d119e6 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -45,6 +45,7 @@
 #include "commands/tablespace.h"
 #include "common/file_utils.h"
 #include "miscadmin.h"
+#include "nodes/miscnodes.h"
 #include "pgstat.h"
 #include "postmaster/bgwriter.h"
 #include "postmaster/startup.h"
diff --git a/src/backend/storage/ipc/waiteventset.c b/src/backend/storage/ipc/waiteventset.c
index 7c0e66900f9..b0746521ae4 100644
--- a/src/backend/storage/ipc/waiteventset.c
+++ b/src/backend/storage/ipc/waiteventset.c
@@ -67,6 +67,7 @@
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
 #include "pgstat.h"
+#include "port/atomics.h"
 #include "portability/instr_time.h"
 #include "postmaster/postmaster.h"
 #include "storage/fd.h"
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index 07a1116671b..a864ae8e6a6 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -25,6 +25,7 @@
 #include "postgres.h"
 
 #include "access/xlog.h"
+#include "executor/instrument.h"
 #include "storage/bufmgr.h"
 #include "storage/proc.h"
 #include "storage/procarray.h"
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f402b17295c..e4a59a30b8c 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -11,6 +11,7 @@
 #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 */
diff --git a/src/include/replication/conflict.h b/src/include/replication/conflict.h
index e516caa5c73..c8fbf9e51b8 100644
--- a/src/include/replication/conflict.h
+++ b/src/include/replication/conflict.h
@@ -9,9 +9,16 @@
 #ifndef CONFLICT_H
 #define CONFLICT_H
 
-#include "nodes/execnodes.h"
+#include "access/xlogdefs.h"
+#include "nodes/pg_list.h"
 #include "utils/timestamp.h"
 
+/* Avoid including execnodes.h here */
+typedef struct EState EState;
+typedef struct ResultRelInfo ResultRelInfo;
+typedef struct TupleTableSlot TupleTableSlot;
+
+
 /*
  * Conflict types that could occur while applying remote changes.
  *
-- 
2.47.3


--mrptdtvcz3f3uhfa--