Feedback-ID: ie3de48e3:Fastmail
Date: Fri, 19 Sep 2025 21:58:53 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: allow benign typedef redefinitions (C11)
Message-ID: <202509191927.uj2ijwmho7nv@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="az6rtwqlicg2ysru"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202509191025.22agk3fvpilc@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/202509191927.uj2ijwmho7nv%40alvherre.pgsql>
Precedence: bulk


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

I happened to realize that brin_tuple.h (a somewhat nasty header which
was only supposed to be used by BRIN itself and stuff like amcheck)
recently somehow snuck into tuplesort.h with some nefarious
consequences: that one has polluted execnodes.h, which means that header
now depends on struct definitions that appear in genam.h.  We should fix
this.  This patch is not the full solution, just a way to show the
problem; for a definitive solution, IMO the definitions of structs
IndexScanInstrumentation and SharedIndexScanInstrumentation should be
elsewhere so that execnodes.h doesn't have to include genam.h.

(gin_tuple.h is also there, but that one's much less of a problem.)

Oh, and replication/conflict.h including all of execnodes.h is kind of
ridiculous.  Looks like that's easily solved with something like this
though:

diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f402b17295c..7bcb4c68e18 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"
 #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..e71b3c8f3d3 100644
--- a/src/include/replication/conflict.h
+++ b/src/include/replication/conflict.h
@@ -9,9 +9,14 @@
 #ifndef CONFLICT_H
 #define CONFLICT_H

-#include "nodes/execnodes.h"
+#include "access/xlogdefs.h"
+#include "nodes/pg_list.h"
 #include "utils/timestamp.h"

+typedef struct EState EState;
+typedef struct ResultRelInfo ResultRelInfo;
+typedef struct TupleTableSlot TupleTableSlot;
+
 /*
  * Conflict types that could occur while applying remote changes.
  *


-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La espina, desde que nace, ya pincha" (Proverbio africano)

--az6rtwqlicg2ysru
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename="nobrintuple.patch"

diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index e9d4b27427e..43c007b6204 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -29,10 +29,11 @@
 #include "storage/bufmgr.h"
 #include "storage/predicate.h"
 #include "tcop/tcopprot.h"
+#include "utils/builtins.h"
 #include "utils/datum.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
-#include "utils/builtins.h"
+#include "utils/typcache.h"
 
 
 /* Magic numbers for parallel state sharing */
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index b409d4ecbf5..ab585aa9819 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -14,6 +14,7 @@
 
 #include "postgres.h"
 
+#include "access/amapi.h"
 #include "access/commit_ts.h"
 #include "access/genam.h"
 #include "access/gist.h"
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index e1979a80c19..eebea694dda 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -37,6 +37,7 @@
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/timestamp.h"
+#include "utils/typcache.h"
 #include "utils/xml.h"
 
 /* GUC parameters */
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index f59046ad620..85f55b75c66 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -29,6 +29,7 @@
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
+#include "utils/typcache.h"
 
 
 static MemoryContext LogicalRepRelMapContext = NULL;
diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index 1db6a7f784c..4e8efcdf55b 100644
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -29,6 +29,7 @@
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
+#include "utils/typcache.h"
 
 #define DEFAULT_NULL_FRAC      Float4GetDatum(0.0)
 #define DEFAULT_AVG_WIDTH      Int32GetDatum(0) /* unknown */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index daa6c44a9ac..b72084b7aaa 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -29,6 +29,7 @@
 #ifndef EXECNODES_H
 #define EXECNODES_H
 
+#include "access/genam.h"		/* FIXME */
 #include "access/tupconvert.h"
 #include "executor/instrument.h"
 #include "fmgr.h"
@@ -59,6 +60,10 @@ struct ExprEvalStep;			/* avoid including execExpr.h everywhere */
 struct CopyMultiInsertBuffer;
 struct LogicalTapeSet;
 
+typedef struct ScanKeyData ScanKeyData;
+typedef struct SharedIndexScanInstrumentation SharedIndexScanInstrumentation;
+typedef struct IndexScanInstrumentation IndexScanInstrumentation;
+
 
 /* ----------------
  *		ExprState node
diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index ef79f259f93..38821b5cdbb 100644
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -21,7 +21,6 @@
 #ifndef TUPLESORT_H
 #define TUPLESORT_H
 
-#include "access/brin_tuple.h"
 #include "access/gin_tuple.h"
 #include "access/itup.h"
 #include "executor/tuptable.h"
@@ -38,6 +37,9 @@
 typedef struct Tuplesortstate Tuplesortstate;
 typedef struct Sharedsort Sharedsort;
 
+/* avoid including brin_tuple.h here */
+typedef struct BrinTuple BrinTuple;
+
 /*
  * Tuplesort parallel coordination state, allocated by each participant in
  * local memory.  Participant caller initializes everything.  See usage notes

--az6rtwqlicg2ysru--