Re: Avoid circular header file dependency - Mailing list pgsql-hackers

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> On Sat, Apr 26, 2025 at 01:20:56AM -0400, Tom Lane wrote:
>> Whatever it contains, we need to kill it with fire before the problem
>> metastasizes like it did the last time.  (yeah, yeah, badly mixed
>> metaphors)  I can take a look at this one over the weekend if nobody
>> beats me to it.

> I had a look at it, what do you think about 0002 attached? (Not 100% sure
> that's the best approach though).

After looking at this, I think the actual problem is that plpython.h
does this:

/*
 * Used throughout, so it's easier to just include it everywhere.
 */
#include "plpy_util.h"

which is exactly the sort of cowboy modularity violation that hurts
when you have to clean it up.  Taking that out requires having to
manually include plpy_util.h in all the relevant .c files, but on
the other hand we can remove their vestigial direct inclusions of
plpython.h.  It was always pretty silly to #include that after
including some plpy_foo.h files, so let's stop doing so.  The attached
patch therefore boils down in most places to s/plpython.h/plpy_util.h/.

(A small number of these files still compiled without that, indicating
that they're not actually using plpy_util.h today.  But I figured we
might as well just do it uniformly.)

            regards, tom lane

diff --git a/contrib/hstore_plpython/hstore_plpython.c b/contrib/hstore_plpython/hstore_plpython.c
index 8812fb3f3e4..e2bfc6da38e 100644
--- a/contrib/hstore_plpython/hstore_plpython.c
+++ b/contrib/hstore_plpython/hstore_plpython.c
@@ -3,7 +3,7 @@
 #include "fmgr.h"
 #include "hstore/hstore.h"
 #include "plpy_typeio.h"
-#include "plpython.h"
+#include "plpy_util.h"

 PG_MODULE_MAGIC_EXT(
                     .name = "hstore_plpython",
diff --git a/contrib/jsonb_plpython/jsonb_plpython.c b/contrib/jsonb_plpython/jsonb_plpython.c
index 680445a006f..9383615abbf 100644
--- a/contrib/jsonb_plpython/jsonb_plpython.c
+++ b/contrib/jsonb_plpython/jsonb_plpython.c
@@ -2,7 +2,7 @@

 #include "plpy_elog.h"
 #include "plpy_typeio.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/fmgrprotos.h"
 #include "utils/jsonb.h"
 #include "utils/numeric.h"
diff --git a/contrib/ltree_plpython/ltree_plpython.c b/contrib/ltree_plpython/ltree_plpython.c
index ba5926b8be6..0493aeb2423 100644
--- a/contrib/ltree_plpython/ltree_plpython.c
+++ b/contrib/ltree_plpython/ltree_plpython.c
@@ -2,7 +2,7 @@

 #include "fmgr.h"
 #include "ltree/ltree.h"
-#include "plpython.h"
+#include "plpy_util.h"

 PG_MODULE_MAGIC_EXT(
                     .name = "ltree_plpython",
diff --git a/src/pl/plpython/plpy_cursorobject.c b/src/pl/plpython/plpy_cursorobject.c
index 1c6be756120..37d7efca77c 100644
--- a/src/pl/plpython/plpy_cursorobject.c
+++ b/src/pl/plpython/plpy_cursorobject.c
@@ -16,7 +16,7 @@
 #include "plpy_planobject.h"
 #include "plpy_resultobject.h"
 #include "plpy_spi.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/memutils.h"

 static PyObject *PLy_cursor_query(const char *query);
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
index 70de5ba13d7..ddf3573f0e7 100644
--- a/src/pl/plpython/plpy_elog.c
+++ b/src/pl/plpython/plpy_elog.c
@@ -10,7 +10,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 #include "plpy_procedure.h"
-#include "plpython.h"
+#include "plpy_util.h"

 PyObject   *PLy_exc_error = NULL;
 PyObject   *PLy_exc_fatal = NULL;
diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index 00747bb811b..28fbd443b98 100644
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -17,7 +17,7 @@
 #include "plpy_main.h"
 #include "plpy_procedure.h"
 #include "plpy_subxactobject.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/fmgrprotos.h"
 #include "utils/rel.h"

diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 8f56155f006..f36eadbadc6 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -18,7 +18,7 @@
 #include "plpy_plpymodule.h"
 #include "plpy_procedure.h"
 #include "plpy_subxactobject.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
diff --git a/src/pl/plpython/plpy_planobject.c b/src/pl/plpython/plpy_planobject.c
index 3e385555e5e..6044893afdd 100644
--- a/src/pl/plpython/plpy_planobject.c
+++ b/src/pl/plpython/plpy_planobject.c
@@ -9,7 +9,7 @@
 #include "plpy_cursorobject.h"
 #include "plpy_planobject.h"
 #include "plpy_spi.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/memutils.h"

 static void PLy_plan_dealloc(PLyPlanObject *self);
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index ea06d9a52b1..1f980b44b2a 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -14,7 +14,7 @@
 #include "plpy_resultobject.h"
 #include "plpy_spi.h"
 #include "plpy_subxactobject.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/builtins.h"

 HTAB       *PLy_spi_exceptions = NULL;
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index b494eeb474f..c176d24e801 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -13,7 +13,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 #include "plpy_procedure.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/builtins.h"
 #include "utils/hsearch.h"
 #include "utils/memutils.h"
diff --git a/src/pl/plpython/plpy_resultobject.c b/src/pl/plpython/plpy_resultobject.c
index 80fa1d7accc..0d9997cbaa3 100644
--- a/src/pl/plpython/plpy_resultobject.c
+++ b/src/pl/plpython/plpy_resultobject.c
@@ -8,7 +8,7 @@

 #include "plpy_elog.h"
 #include "plpy_resultobject.h"
-#include "plpython.h"
+#include "plpy_util.h"

 static void PLy_result_dealloc(PLyResultObject *self);
 static PyObject *PLy_result_colnames(PyObject *self, PyObject *unused);
diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index 77fbfd6c868..1e386aadcca 100644
--- a/src/pl/plpython/plpy_spi.c
+++ b/src/pl/plpython/plpy_spi.c
@@ -19,7 +19,7 @@
 #include "plpy_plpymodule.h"
 #include "plpy_resultobject.h"
 #include "plpy_spi.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/memutils.h"

 static PyObject *PLy_spi_execute_query(char *query, long limit);
diff --git a/src/pl/plpython/plpy_subxactobject.c b/src/pl/plpython/plpy_subxactobject.c
index ad24a9fc4b8..c2484a99b4a 100644
--- a/src/pl/plpython/plpy_subxactobject.c
+++ b/src/pl/plpython/plpy_subxactobject.c
@@ -9,7 +9,7 @@
 #include "access/xact.h"
 #include "plpy_elog.h"
 #include "plpy_subxactobject.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/memutils.h"

 List       *explicit_subtransactions = NIL;
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 1d127ae3ffe..f6509a41902 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -14,7 +14,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 #include "plpy_typeio.h"
-#include "plpython.h"
+#include "plpy_util.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c
index 6d89b1cb60a..ef710aa371e 100644
--- a/src/pl/plpython/plpy_util.c
+++ b/src/pl/plpython/plpy_util.c
@@ -9,7 +9,6 @@
 #include "mb/pg_wchar.h"
 #include "plpy_elog.h"
 #include "plpy_util.h"
-#include "plpython.h"

 /*
  * Convert a Python unicode object to a Python string/bytes object in
diff --git a/src/pl/plpython/plpython.h b/src/pl/plpython/plpython.h
index 06fc1a5440f..118b3100840 100644
--- a/src/pl/plpython/plpython.h
+++ b/src/pl/plpython/plpython.h
@@ -2,6 +2,10 @@
  *
  * plpython.h - Python as a procedural language for PostgreSQL
  *
+ * Note: this file is #include'd by each of the sub-module header files
+ * (plpy_elog.h, etc).  It's therefore unnecessary for any plpython *.c
+ * files to include it directly.
+ *
  * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
@@ -38,9 +42,4 @@
 #undef TEXTDOMAIN
 #define TEXTDOMAIN PG_TEXTDOMAIN("plpython")

-/*
- * Used throughout, so it's easier to just include it everywhere.
- */
-#include "plpy_util.h"
-
 #endif                            /* PLPYTHON_H */