Re: Add Postgres module info - Mailing list pgsql-hackers

I spent awhile reviewing the v5 patch, and here's a proposed v6.
Some notes:

* I didn't like depending on offsetof(Pg_magic_struct, module_extra)
to determine which parts of the struct are checked for compatibility.
It just seems way too easy to break that with careless insertion
of new fields, and such breakage might not cause obvious failures.
I think the right thing is to break out the ABI-checking fields as
their own sub-struct, rather than breaking out the new fields as a
sub-struct.

* I renamed the inquiry function to pg_get_loaded_modules, since
it only works on loaded modules but that's hardly clear from the
previous name.

* It is not clear to me what permission restrictions we should put
on pg_get_loaded_modules, but it is clear that "none" is the wrong
answer.  In particular, exposing the full file path of loaded modules
is against our rules: unprivileged users are not supposed to be able
to learn anything about the filesystem underneath the server.  (This
is why for instance an unprivileged user can't read the data_directory
GUC.)  In the attached I made the library path read as NULL unless the
user has pg_read_server_files, but I'm not attached to that specific
solution.  One thing not to like is that it's very likely that you'd
just get a row of NULLs and no useful info about a module at all.
Another idea perhaps could be to strip off the directory path and
maybe the filename extension if the user doesn't have privilege.
Or we could remove the internal permission check and instead gate
access to the function altogether with grantable EXECUTE privilege.
(This might be the right answer, since it's not clear that Joe
Unprivileged User should be able to know what modules are loaded; some
of them might have security implications.)  In any case, requiring
pg_read_server_files feels a little too strong, but I don't see an
alternative role I like better.  The EXECUTE-privilege answer would at
least let installations adjust the function's availability to their
liking.

* I didn't like anything about the test setup.  Making test_misc
dependent on other modules is a recipe for confusion, and perhaps for
failures in parallel builds.  (Yes, I see somebody already made it
depend on injection_points.  But doubling down on a bad idea doesn't
make it less bad.)  Also, the test would fail completely in an
installation that came with any preloaded modules, which hardly seems
like an improbable future situation.  I think we need to restrict what
modules we're looking at with a WHERE clause to prevent that.  After
some thought I went back to the upthread idea of just having
auto_explain as a test case.

Still TBD:

* I'm not happy with putting pg_get_loaded_modules into dfmgr.c.
It feels like the wrong layer to have a SQL-callable function,
and the large expansion in its #include list is evidence that we're
adding functionality that doesn't belong there.  But I'm not quite
sure where to put it instead.  Also, the naive way to do that would
require exporting DynamicFileList which doesn't feel nice either.
Maybe we could make dfmgr.c export some sort of iterator function?

* Should we convert our existing modules to use PG_MODULE_MAGIC_EXT?
I'm mildly in favor of that, but I think we'd need some automated way
to manage their version strings, and I don't know what that ought to
look like.  Maybe it'd be enough to make all the in-core modules use
PG_VERSION as their version string, but I think that might put a dent
in the idea of the version strings following semantic versioning
rules.

            regards, tom lane

From 2f8e182dbd26e03a9568393307b64cb26b3842fd Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 22 Mar 2025 18:39:15 -0400
Subject: [PATCH v6] Introduce PG_MODULE_MAGIC_EXT macro.

This macro allows dynamically loaded shared libraries (modules) to
provide a wired-in module name and version, and possibly other
compile-time-constant fields in future.  This information can be
retrieved with the new pg_get_loaded_modules() function.

This feature is expected to be particularly useful for modules
that do not have any exposed SQL functionality and thus are
not associated with a SQL-level extension object.  But even for
modules that do belong to extensions, being able to verify the
actual code version can be useful.

Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Yurii Rashkovskii <yrashk@omnigres.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/dd4d1b59-d0fe-49d5-b28f-1e463b68fa32@gmail.com
---
 contrib/auto_explain/auto_explain.c        |  5 +-
 contrib/auto_explain/t/001_auto_explain.pl | 11 +++
 doc/src/sgml/func.sgml                     | 24 +++++++
 doc/src/sgml/xfunc.sgml                    | 24 +++++++
 src/backend/utils/fmgr/dfmgr.c             | 81 ++++++++++++++++++++--
 src/include/catalog/pg_proc.dat            |  7 ++
 src/include/fmgr.h                         | 62 ++++++++++++++---
 src/tools/pgindent/typedefs.list           |  1 +
 8 files changed, 196 insertions(+), 19 deletions(-)

diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 3b73bd19107..72615a3c116 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -22,7 +22,10 @@
 #include "executor/instrument.h"
 #include "utils/guc.h"

-PG_MODULE_MAGIC;
+PG_MODULE_MAGIC_EXT(
+                    .name = "auto_explain",
+                    .version = "1.0.0"
+);

 /* GUC variables */
 static int    auto_explain_log_min_duration = -1; /* msec or -1 */
diff --git a/contrib/auto_explain/t/001_auto_explain.pl b/contrib/auto_explain/t/001_auto_explain.pl
index 25252604b7d..07692b20e71 100644
--- a/contrib/auto_explain/t/001_auto_explain.pl
+++ b/contrib/auto_explain/t/001_auto_explain.pl
@@ -212,4 +212,15 @@ REVOKE SET ON PARAMETER auto_explain.log_format FROM regress_user1;
 DROP USER regress_user1;
 });

+# Test pg_get_loaded_modules() function.  This function is particularly
+# useful for modules with no SQL presence, such as auto_explain.
+
+my $res = $node->safe_psql(
+    "postgres", q{
+SELECT module_name, version, library_path != '' as library_path_provided
+FROM pg_get_loaded_modules()
+WHERE module_name = 'auto_explain';
+});
+like($res, qr/^auto_explain\|1\.0\.0\|t$/, "pg_get_loaded_modules() ok");
+
 done_testing();
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 6fa1d6586b8..306f1be2715 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25002,6 +25002,30 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
        </para></entry>
       </row>

+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_get_loaded_modules</primary>
+        </indexterm>
+        <function>pg_get_loaded_modules</function> ()
+        <returnvalue>setof record</returnvalue>
+        ( <parameter>module_name</parameter> <type>text</type>,
+        <parameter>version</parameter> <type>text</type>,
+        <parameter>library_path</parameter> <type>text</type> )
+       </para>
+       <para>
+        Returns a list of the loadable modules that are loaded into the
+        current server session.  The <parameter>module_name</parameter>
+        and <parameter>version</parameter> fields are NULL unless the
+        module author supplied values for them using
+        the <literal>PG_MODULE_MAGIC_EXT</literal> macro.
+        The <parameter>library_path</parameter> field gives the full path
+        name of the loaded module, but it is NULL if the user does not
+        have the privileges of the <literal>pg_read_server_files</literal>
+        role.
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 9f22dacac7d..0f8f6040d9b 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -1968,6 +1968,30 @@ CREATE FUNCTION square_root(double precision) RETURNS double precision
 <programlisting>
 PG_MODULE_MAGIC;
 </programlisting>
+or
+<programlisting>
+PG_MODULE_MAGIC_EXT(<replaceable>parameters</replaceable>);
+</programlisting>
+   </para>
+
+   <para>
+    The <literal>PG_MODULE_MAGIC_EXT</literal> macro allows the specification
+    of additional information about the module; currently, a name and/or a
+    version string can be added.  (More fields might be allowed in future.)
+    Write something like this:
+
+<programlisting>
+PG_MODULE_MAGIC_EXT(
+    .name = "my_module_name",
+    .version = "1.2.3"
+);
+</programlisting>
+
+    Subsequently the name and version can be examined via
+    the <function>pg_get_loaded_modules()</function> function.
+    The meaning of the version string is not restricted
+    by <productname>PostgreSQL</productname>, but use of semantic versioning
+    rules is recommended.
    </para>

    <para>
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index dd4c83d1bba..e430410d080 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -20,11 +20,15 @@
 #include <dlfcn.h>
 #endif                            /* !WIN32 */

+#include "catalog/pg_authid_d.h"
 #include "fmgr.h"
+#include "funcapi.h"
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/shmem.h"
+#include "utils/acl.h"
+#include "utils/builtins.h"
 #include "utils/hsearch.h"


@@ -51,6 +55,7 @@ typedef struct df_files
     ino_t        inode;            /* Inode number of file */
 #endif
     void       *handle;            /* a handle for pg_dl* functions */
+    const Pg_magic_struct *magic;    /* Location of module's magic block */
     char        filename[FLEXIBLE_ARRAY_MEMBER];    /* Full pathname of file */
 } DynamicFileList;

@@ -68,12 +73,12 @@ char       *Dynamic_library_path;

 static void *internal_load_library(const char *libname);
 pg_noreturn static void incompatible_module_error(const char *libname,
-                                                  const Pg_magic_struct *module_magic_data);
+                                                  const Pg_abi_values *module_magic_data);
 static char *expand_dynamic_library_name(const char *name);
 static void check_restricted_library_name(const char *name);

-/* Magic structure that module needs to match to be accepted */
-static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
+/* ABI values that module needs to match to be accepted */
+static const Pg_abi_values magic_data = PG_MODULE_ABI_DATA;


 /*
@@ -243,8 +248,10 @@ internal_load_library(const char *libname)
         {
             const Pg_magic_struct *magic_data_ptr = (*magic_func) ();

-            if (magic_data_ptr->len != magic_data.len ||
-                memcmp(magic_data_ptr, &magic_data, magic_data.len) != 0)
+            /* Check ABI compatibility fields */
+            if (magic_data_ptr->len != sizeof(Pg_magic_struct) ||
+                memcmp(&magic_data_ptr->abi_fields, &magic_data,
+                       sizeof(Pg_abi_values)) != 0)
             {
                 /* copy data block before unlinking library */
                 Pg_magic_struct module_magic_data = *magic_data_ptr;
@@ -254,8 +261,11 @@ internal_load_library(const char *libname)
                 free(file_scanner);

                 /* issue suitable complaint */
-                incompatible_module_error(libname, &module_magic_data);
+                incompatible_module_error(libname, &module_magic_data.abi_fields);
             }
+
+            /* Remember the magic block's location for future use */
+            file_scanner->magic = magic_data_ptr;
         }
         else
         {
@@ -292,7 +302,7 @@ internal_load_library(const char *libname)
  */
 static void
 incompatible_module_error(const char *libname,
-                          const Pg_magic_struct *module_magic_data)
+                          const Pg_abi_values *module_magic_data)
 {
     StringInfoData details;

@@ -694,3 +704,60 @@ RestoreLibraryState(char *start_address)
         start_address += strlen(start_address) + 1;
     }
 }
+
+/*
+ * SQL-callable function to get per-loaded-module information.
+ */
+Datum
+pg_get_loaded_modules(PG_FUNCTION_ARGS)
+{
+    FuncCallContext *funcctx;
+    MemoryContext oldcontext;
+    DynamicFileList *file_scanner;
+    Datum        result;
+    Datum        values[3];
+    bool        isnull[3] = {0};
+
+    if (SRF_IS_FIRSTCALL())
+    {
+        TupleDesc    tupdesc;
+
+        funcctx = SRF_FIRSTCALL_INIT();
+        oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+        if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+            elog(ERROR, "return type must be a row type");
+
+        funcctx->tuple_desc = BlessTupleDesc(tupdesc);
+        funcctx->user_fctx = (void *) file_list;
+
+        MemoryContextSwitchTo(oldcontext);
+    }
+
+    funcctx = SRF_PERCALL_SETUP();
+    file_scanner = (DynamicFileList *) funcctx->user_fctx;
+
+    if (file_scanner != NULL)
+    {
+        if (file_scanner->magic->name == NULL)
+            isnull[0] = true;
+        else
+            values[0] = CStringGetTextDatum(file_scanner->magic->name);
+        if (file_scanner->magic->version == NULL)
+            isnull[1] = true;
+        else
+            values[1] = CStringGetTextDatum(file_scanner->magic->version);
+        if (!has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
+            isnull[2] = true;
+        else
+            values[2] = CStringGetTextDatum(file_scanner->filename);
+
+        result = HeapTupleGetDatum(heap_form_tuple(funcctx->tuple_desc,
+                                                   values, isnull));
+
+        funcctx->user_fctx = file_scanner->next;
+        SRF_RETURN_NEXT(funcctx, result);
+    }
+    else
+        SRF_RETURN_DONE(funcctx);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 890822eaf79..86389afa3f2 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6749,6 +6749,13 @@
   proargnames => '{rm_id, rm_name, rm_builtin}',
   prosrc => 'pg_get_wal_resource_managers' },

+{ oid => '8303', descr => 'get info about loaded modules',
+  proname => 'pg_get_loaded_modules', prorows => '10', proretset => 't',
+  provolatile => 'v', proparallel => 'r', prorettype => 'record',
+  proargtypes => '', proallargtypes => '{text,text,text}',
+  proargmodes => '{o,o,o}', proargnames => '{module_name,version,library_path}',
+  prosrc => 'pg_get_loaded_modules' },
+
 { oid => '2621', descr => 'reload configuration files',
   proname => 'pg_reload_conf', provolatile => 'v', prorettype => 'bool',
   proargtypes => '', prosrc => 'pg_reload_conf' },
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 82ee38b31e5..c6dbb76d68e 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -440,11 +440,14 @@ extern PGDLLEXPORT void _PG_init(void);
  * We require dynamically-loaded modules to include the macro call
  *        PG_MODULE_MAGIC;
  * so that we can check for obvious incompatibility, such as being compiled
- * for a different major PostgreSQL version.
+ * for a different major PostgreSQL version.  Alternatively, write
+ *        PG_MODULE_MAGIC_EXT(...);
+ * where the optional arguments can specify module name and version, and
+ * perhaps other values in future.  Note that in a multiple-source-file
+ * module, there should be exactly one such macro call.
  *
- * To compile with versions of PostgreSQL that do not support this,
- * you may put an #ifdef/#endif test around it.  Note that in a multiple-
- * source-file module, the macro call should only appear once.
+ * You may need an #ifdef test to verify that the version of PostgreSQL
+ * you are compiling against supports PG_MODULE_MAGIC_EXT().
  *
  * The specific items included in the magic block are intended to be ones that
  * are custom-configurable and especially likely to break dynamically loaded
@@ -459,22 +462,30 @@ extern PGDLLEXPORT void _PG_init(void);
  *-------------------------------------------------------------------------
  */

-/* Definition of the magic block structure */
+/* Definition of the values we check to verify ABI compatibility */
 typedef struct
 {
-    int            len;            /* sizeof(this struct) */
     int            version;        /* PostgreSQL major version */
     int            funcmaxargs;    /* FUNC_MAX_ARGS */
     int            indexmaxkeys;    /* INDEX_MAX_KEYS */
     int            namedatalen;    /* NAMEDATALEN */
     int            float8byval;    /* FLOAT8PASSBYVAL */
     char        abi_extra[32];    /* see pg_config_manual.h */
+} Pg_abi_values;
+
+/* Definition of the magic block structure */
+typedef struct
+{
+    int            len;            /* sizeof(this struct) */
+    Pg_abi_values abi_fields;    /* see above */
+    /* Remaining fields are zero unless filled via PG_MODULE_MAGIC_EXT */
+    const char *name;            /* optional module name */
+    const char *version;        /* optional module version */
 } Pg_magic_struct;

-/* The actual data block contents */
-#define PG_MODULE_MAGIC_DATA \
+/* Macro to fill the ABI fields */
+#define PG_MODULE_ABI_DATA \
 { \
-    sizeof(Pg_magic_struct), \
     PG_VERSION_NUM / 100, \
     FUNC_MAX_ARGS, \
     INDEX_MAX_KEYS, \
@@ -483,7 +494,18 @@ typedef struct
     FMGR_ABI_EXTRA, \
 }

-StaticAssertDecl(sizeof(FMGR_ABI_EXTRA) <= sizeof(((Pg_magic_struct *) 0)->abi_extra),
+/*
+ * Macro to fill a magic block.  If any arguments are given, they should
+ * be field initializers.
+ */
+#define PG_MODULE_MAGIC_DATA(...) \
+{ \
+    .len = sizeof(Pg_magic_struct), \
+    .abi_fields = PG_MODULE_ABI_DATA, \
+    __VA_ARGS__ \
+}
+
+StaticAssertDecl(sizeof(FMGR_ABI_EXTRA) <= sizeof(((Pg_abi_values *) 0)->abi_extra),
                  "FMGR_ABI_EXTRA too long");

 /*
@@ -500,11 +522,29 @@ extern PGDLLEXPORT const Pg_magic_struct *PG_MAGIC_FUNCTION_NAME(void); \
 const Pg_magic_struct * \
 PG_MAGIC_FUNCTION_NAME(void) \
 { \
-    static const Pg_magic_struct Pg_magic_data = PG_MODULE_MAGIC_DATA; \
+    static const Pg_magic_struct Pg_magic_data = PG_MODULE_MAGIC_DATA(0); \
     return &Pg_magic_data; \
 } \
 extern int no_such_variable

+/*
+ * Alternate declaration that allows specification of additional fields.
+ * The additional values should be written as field initializers, for example
+ *    PG_MODULE_MAGIC_EXT(
+ *        .name = "some string",
+ *        .version = "some string"
+ *    );
+ */
+#define PG_MODULE_MAGIC_EXT(...) \
+extern PGDLLEXPORT const Pg_magic_struct *PG_MAGIC_FUNCTION_NAME(void); \
+const Pg_magic_struct * \
+PG_MAGIC_FUNCTION_NAME(void) \
+{ \
+    static const Pg_magic_struct Pg_magic_data = \
+        PG_MODULE_MAGIC_DATA(__VA_ARGS__); \
+    return &Pg_magic_data; \
+} \
+extern int no_such_variable

 /*-------------------------------------------------------------------------
  *        Support routines and macros for callers of fmgr-compatible functions
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index bfa276d2d35..854b16d0dcc 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2228,6 +2228,7 @@ PgStat_WalCounters
 PgStat_WalStats
 PgXmlErrorContext
 PgXmlStrictness
+Pg_abi_values
 Pg_finfo_record
 Pg_magic_struct
 PipeProtoChunk
--
2.43.5