Hi,
On Sat, Apr 26, 2025 at 01:20:56AM -0400, Tom Lane wrote:
> Bertrand Drouvot <bertranddrouvot...@gmail.com> writes:
> > While working on wait events I faced some compilation issues due to circular
> > header file dependency (introduced in fa88928470b5) between wait_event.h and
> > wait_event_types.h.
>
> I don't have an opinion about whether this
> specific refactoring is the best way to deal with this case, but
> I definitely feel that we mustn't allow the situation to persist.
Thanks for sharing your thoughts! yeah, I'm not 100% sure that the proposed
refactoring is the best way but it looks simple enough and allows
wait_event_types.h to include "only" what it really needs.
> > Out of curiosity, I ran clang-tidy with misc-header-include-cycle ([1]) and
> > it
> > also reports:
> > ../src/pl/plpython/plpy_util.h:9:10: warning: circular header file
> > dependency detected while including 'plpython.h'
> > This one worries me less because plpy_util.h only contains simple external
> > function declarations.
>
> 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).
> I am very glad to hear that there's a readily available tool to
> catch such cases.
+1, and it's good to see that there are only 2 cases in the code base.
> We ought to run it every so often.
Yeah, also probably with readability-inconsistent-declaration-parameter-name
that
Peter used in [1].
How/where do we "automate" this kind of regular tasks? (I tried to find the
answer
in [2] but that thread is pretty long).
[1]:
https://www.postgresql.org/message-id/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg%40mail.gmail.com
[2]:
https://www.postgresql.org/message-id/flat/20200812223409.6di3y2qsnvynao7a%40alap3.anarazel.de
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From 2156bf212ce7aabc40e2843c77d796249d12d4f9 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot...@gmail.com>
Date: Thu, 24 Apr 2025 17:11:03 +0000
Subject: [PATCH v1 1/2] Avoid including wait_event.h in wait_event_types.h
Adding wait_class_constants.h to avoid including wait_event.h in
wait_event_types.h (that produced a circular include).
---
.../activity/generate-wait_event_types.pl | 2 +-
src/include/utils/wait_class_constants.h | 29 +++++++++++++++++++
src/include/utils/wait_event.h | 17 ++---------
3 files changed, 32 insertions(+), 16 deletions(-)
create mode 100644 src/include/utils/wait_class_constants.h
diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index 171bf2ae632..52b6b7b2ae6 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -168,7 +168,7 @@ if ($gen_code)
printf $h $header_comment, 'wait_event_types.h';
printf $h "#ifndef WAIT_EVENT_TYPES_H\n";
printf $h "#define WAIT_EVENT_TYPES_H\n\n";
- printf $h "#include \"utils/wait_event.h\"\n\n";
+ printf $h "#include \"utils/wait_class_constants.h\"\n\n";
printf $c $header_comment, 'pgstat_wait_event.c';
diff --git a/src/include/utils/wait_class_constants.h b/src/include/utils/wait_class_constants.h
new file mode 100644
index 00000000000..286f9c2daec
--- /dev/null
+++ b/src/include/utils/wait_class_constants.h
@@ -0,0 +1,29 @@
+/*-------------------------------------------------------------------------
+ * wait_class_constants.h
+ * Constants related to wait classes
+ *
+ * Copyright (c) 2001-2025, PostgreSQL Global Development Group
+ *
+ * src/include/utils/wait_class_constants.h
+ * ----------
+ */
+#ifndef WAIT_CLASS_CONSTANTS_H
+#define WAIT_CLASS_CONSTANTS_H
+
+
+/* ----------
+ * Wait Classes
+ * ----------
+ */
+#define PG_WAIT_LWLOCK 0x01000000U
+#define PG_WAIT_LOCK 0x03000000U
+#define PG_WAIT_BUFFERPIN 0x04000000U
+#define PG_WAIT_ACTIVITY 0x05000000U
+#define PG_WAIT_CLIENT 0x06000000U
+#define PG_WAIT_EXTENSION 0x07000000U
+#define PG_WAIT_IPC 0x08000000U
+#define PG_WAIT_TIMEOUT 0x09000000U
+#define PG_WAIT_IO 0x0A000000U
+#define PG_WAIT_INJECTIONPOINT 0x0B000000U
+
+#endif /* WAIT_CLASS_CONSTANTS_H */
diff --git a/src/include/utils/wait_event.h b/src/include/utils/wait_event.h
index b8cb3e5a430..c3c1ce6ef2e 100644
--- a/src/include/utils/wait_event.h
+++ b/src/include/utils/wait_event.h
@@ -10,21 +10,8 @@
#ifndef WAIT_EVENT_H
#define WAIT_EVENT_H
-
-/* ----------
- * Wait Classes
- * ----------
- */
-#define PG_WAIT_LWLOCK 0x01000000U
-#define PG_WAIT_LOCK 0x03000000U
-#define PG_WAIT_BUFFERPIN 0x04000000U
-#define PG_WAIT_ACTIVITY 0x05000000U
-#define PG_WAIT_CLIENT 0x06000000U
-#define PG_WAIT_EXTENSION 0x07000000U
-#define PG_WAIT_IPC 0x08000000U
-#define PG_WAIT_TIMEOUT 0x09000000U
-#define PG_WAIT_IO 0x0A000000U
-#define PG_WAIT_INJECTIONPOINT 0x0B000000U
+/* wait class constants */
+#include "utils/wait_class_constants.h"
/* enums for wait events */
#include "utils/wait_event_types.h"
--
2.34.1
>From 51ddf8b036dd167a29c56632b8855d27fd361838 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot...@gmail.com>
Date: Sat, 26 Apr 2025 07:14:37 +0000
Subject: [PATCH v1 2/2] Avoid including plpython.h in plpy_util.h
Adding plpython_base.h to avoid including plpython.h in plpy_util.h (that
produced a circular include).
---
src/pl/plpython/plpy_util.h | 2 +-
src/pl/plpython/plpython.h | 26 +--------------------
src/pl/plpython/plpython_base.h | 41 +++++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+), 26 deletions(-)
create mode 100644 src/pl/plpython/plpython_base.h
diff --git a/src/pl/plpython/plpy_util.h b/src/pl/plpython/plpy_util.h
index 6f491b0f95b..797692a9eec 100644
--- a/src/pl/plpython/plpy_util.h
+++ b/src/pl/plpython/plpy_util.h
@@ -6,7 +6,7 @@
#ifndef PLPY_UTIL_H
#define PLPY_UTIL_H
-#include "plpython.h"
+#include "plpython_base.h"
extern PGDLLEXPORT PyObject *PLyUnicode_Bytes(PyObject *unicode);
extern PGDLLEXPORT char *PLyUnicode_AsString(PyObject *unicode);
diff --git a/src/pl/plpython/plpython.h b/src/pl/plpython/plpython.h
index 06fc1a5440f..fddcc96acdc 100644
--- a/src/pl/plpython/plpython.h
+++ b/src/pl/plpython/plpython.h
@@ -12,31 +12,7 @@
#ifndef PLPYTHON_H
#define PLPYTHON_H
-/* postgres.h needs to be included before Python.h, as usual */
-#if !defined(POSTGRES_H)
-#error postgres.h must be included before plpython.h
-#elif defined(Py_PYTHON_H)
-#error Python.h must be included via plpython.h
-#endif
-
-/*
- * Enable Python Limited API
- *
- * XXX currently not enabled on MSVC because of build failures
- */
-#if !defined(_MSC_VER)
-#define Py_LIMITED_API 0x03020000
-#endif
-
-/*
- * Pull in Python headers via a wrapper header, to control the scope of
- * the system_header pragma therein.
- */
-#include "plpython_system.h"
-
-/* define our text domain for translations */
-#undef TEXTDOMAIN
-#define TEXTDOMAIN PG_TEXTDOMAIN("plpython")
+#include "plpython_base.h"
/*
* Used throughout, so it's easier to just include it everywhere.
diff --git a/src/pl/plpython/plpython_base.h b/src/pl/plpython/plpython_base.h
new file mode 100644
index 00000000000..7abbcb8b233
--- /dev/null
+++ b/src/pl/plpython/plpython_base.h
@@ -0,0 +1,41 @@
+/*-------------------------------------------------------------------------
+ *
+ * plpython_base.h - Python as a procedural language for PostgreSQL
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/pl/plpython/plpython_base.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef PLPYTHON_BASE_H
+#define PLPYTHON_BASE_H
+
+/* postgres.h needs to be included before Python.h, as usual */
+#if !defined(POSTGRES_H)
+#error postgres.h must be included before plpython_base.h
+#elif defined(Py_PYTHON_H)
+#error Python.h must be included via plpython_base.h
+#endif
+
+/*
+ * Enable Python Limited API
+ *
+ * XXX currently not enabled on MSVC because of build failures
+ */
+#if !defined(_MSC_VER)
+#define Py_LIMITED_API 0x03020000
+#endif
+
+/*
+ * Pull in Python headers via a wrapper header, to control the scope of
+ * the system_header pragma therein.
+ */
+#include "plpython_system.h"
+
+/* define our text domain for translations */
+#undef TEXTDOMAIN
+#define TEXTDOMAIN PG_TEXTDOMAIN("plpython")
+
+#endif /* PLPYTHON_BASE_H */
--
2.34.1
