On 17/2/2025 04:00, Michael Paquier wrote:
On Mon, Feb 17, 2025 at 03:41:56AM +0200, Alexander Korotkov wrote:
1) Is it intended to switch all in-core libraries to use PG_MODULE_MAGIC_EXT()?
2) Once we have module version information, it looks natural to
specify the required version for dependant objects, e.g. SQL-funcions
implemented in shared libraries. For instance,
CREATE FUNCTION ... AS 'MODULE_PATHNAME' LANGUAGE C module_version >= '1.0';
For this, and probably other purposes, it's desirable for version to
be something comparable at SQL level. Should we add some builtin
analogue of pg_text_semver?
I see that this is just a way for extensions to map to some data
statically stored in the modules themselves based on what I can see at
[1]. Why not.
+ bool isnull[3] = {0,0,0};
Could be a simpler {0}.
Done
-PG_MODULE_MAGIC;
+PG_MODULE_MAGIC_EXT(
+ .name = "auto_explain",
+ .version = "1.0.0"
+);
It does not make sense to me to stick that into into of the contrib
modules officially supported just for the sake of the API. I'd
Done
suggest to switch in one of the modules of src/test/modules/ that are
loaded with shared_preload_libraries. A second thing I would suggest
to check is a SQL call with a library loaded via SQL with a LOAD.
test_oat_hooks is already LOAD'ed in a couple of scripts, for example.
For the shared_preload_libraries can, you could choose anything to
prove your point with tests.
Done
+Datum
+module_info(PG_FUNCTION_ARGS)
This should use a "pg_" prefix, should use a plural term as it is a
SRF returning information about all the modules loaded. Perhaps just
name it to pg_get_modules() and also map it to a new system view?
Sure, done.
Some problems with `git diff --check\` showing up here.
Done
No documentation provided.
Ok, I haven't been sure this idea has a chance to be committed. I will
introduce the docs in the next version.
--
regards, Andrei Lepikhov
From b7472026bbf2ef49df164492a19d8433cbaa1d12 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <lepi...@gmail.com>
Date: Tue, 19 Nov 2024 18:45:36 +0700
Subject: [PATCH v3] Introduce PG_MODULE_MAGIC_EXT macro.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This macro provides dynamically loaded shared libraries (modules) with standard
way to incorporate version (supposedly, defined according to semantic versioning
specification) and name data. The introduced catalogue routine pg_get_modules≈
can
be used to find this module by name and check the version. It makes users
independent from file naming conventions.
With a growing number of Postgres core hooks and the introduction of named DSM
segments, the number of modules that don't need to be loaded on startup may
grow fast. Moreover, in many cases related to query tree transformation or
extra path recommendation, such modules might not need database objects except
GUCs - see auto_explain as an example. That means they don't need to execute
the 'CREATE EXTENSION' statement at all and don't have a record in
the pg_extension table. Such a trick provides much flexibility, including
an online upgrade and may become widespread.
In addition, it is also convenient in support to be sure that the installation
(or at least the backend) includes a specific version of the module. Even if
a module has an installation script, it is not rare that it provides
an implementation for a range of UI routine versions. It makes sense to ensure
which specific version of the code is used.
Discussions [1,2] already mentioned module-info stuff, but at that time,
extensibility techniques and extension popularity were low, and it wasn't
necessary to provide that data.
[1] https://www.postgresql.org/message-id/flat/20060507211705.GB3808%40svana.org
[2]
https://www.postgresql.org/message-id/flat/20051106162658.34c31d57%40thunder.logicalchaos.org
---
src/backend/utils/fmgr/dfmgr.c | 62 ++++++++++++++++++-
src/include/catalog/pg_proc.dat | 6 ++
src/include/fmgr.h | 28 ++++++++-
.../modules/test_dsa/expected/test_dsa.out | 8 +++
src/test/modules/test_dsa/sql/test_dsa.sql | 4 ++
.../test_shm_mq/expected/test_shm_mq.out | 8 +++
.../modules/test_shm_mq/sql/test_shm_mq.sql | 4 ++
src/test/modules/test_shm_mq/test.c | 5 +-
.../modules/test_slru/expected/test_slru.out | 16 +++++
src/test/modules/test_slru/sql/test_slru.sql | 8 +++
src/test/modules/test_slru/test_slru.c | 5 +-
11 files changed, 147 insertions(+), 7 deletions(-)
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index 87b233cb887..0ae9b21c2bc 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -21,10 +21,12 @@
#endif /* !WIN32 */
#include "fmgr.h"
+#include "funcapi.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "storage/fd.h"
#include "storage/shmem.h"
+#include "utils/builtins.h"
#include "utils/hsearch.h"
@@ -51,6 +53,7 @@ typedef struct df_files
ino_t inode; /* Inode number of file */
#endif
void *handle; /* a handle for pg_dl*
functions */
+ const pg_module_info *minfo;
char filename[FLEXIBLE_ARRAY_MEMBER]; /* Full
pathname of file */
} DynamicFileList;
@@ -75,7 +78,7 @@ static char *substitute_libpath_macro(const char *name);
static char *find_in_dynamic_libpath(const char *basename);
/* Magic structure that module needs to match to be accepted */
-static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
+static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA(0);
/*
@@ -245,8 +248,14 @@ internal_load_library(const char *libname)
{
const Pg_magic_struct *magic_data_ptr = (*magic_func)
();
+ /*
+ * Check magic field from loading library to be sure it
compiled
+ * for the same Postgres code. Skip maintainer fields
at the end
+ * of the struct.
+ */
if (magic_data_ptr->len != magic_data.len ||
- memcmp(magic_data_ptr, &magic_data,
magic_data.len) != 0)
+ memcmp(magic_data_ptr, &magic_data,
+ offsetof(Pg_magic_struct,
module_extra)) != 0)
{
/* copy data block before unlinking library */
Pg_magic_struct module_magic_data =
*magic_data_ptr;
@@ -258,6 +267,9 @@ internal_load_library(const char *libname)
/* issue suitable complaint */
incompatible_module_error(libname,
&module_magic_data);
}
+
+ /* Save link to the maintainer-provided info */
+ file_scanner->minfo = &magic_data_ptr->module_extra;
}
else
{
@@ -675,3 +687,49 @@ RestoreLibraryState(char *start_address)
start_address += strlen(start_address) + 1;
}
}
+
+Datum
+pg_get_modules(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ MemoryContext oldcontext;
+ DynamicFileList *file_scanner = NULL;
+ TupleDesc tupdesc;
+ Datum result;
+ Datum values[3];
+ bool isnull[3] = {0};
+
+ if (SRF_IS_FIRSTCALL())
+ {
+ 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");
+
+ file_scanner = file_list;
+
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ funcctx = SRF_PERCALL_SETUP();
+
+ if (file_scanner != NULL)
+ {
+ if (file_scanner->minfo->name == NULL)
+ isnull[0] = true;
+ else
+ values[0] =
CStringGetTextDatum(file_scanner->minfo->name);
+ if (file_scanner->minfo->version == NULL)
+ isnull[1] = true;
+ else
+ values[1] =
CStringGetTextDatum(file_scanner->minfo->version);
+
+ values[2] = CStringGetTextDatum(file_scanner->filename);
+ result = HeapTupleGetDatum(heap_form_tuple(tupdesc, values,
isnull));
+ file_scanner = 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 cd9422d0bac..8e230f7ce2b 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -311,6 +311,12 @@
proname => 'scalargtjoinsel', provolatile => 's', prorettype => 'float8',
proargtypes => 'internal oid internal int2 internal',
prosrc => 'scalargtjoinsel' },
+{ oid => '111', descr => 'Module Info',
+ proname => 'pg_get_modules', provolatile => 's', prorettype => 'record',
+ proretset => 't', proargtypes => '', proallargtypes => '{text,text,text}',
+ proargmodes => '{o,o,o}', prorows => 10,
+ proargnames => '{module_name,version,libname}',
+ prosrc => 'pg_get_modules' },
{ oid => '336',
descr => 'restriction selectivity of <= and related operators on scalar
datatypes',
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index e609ea875a7..5018abf3a18 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -459,6 +459,12 @@ extern PGDLLEXPORT void _PG_init(void);
*-------------------------------------------------------------------------
*/
+typedef struct pg_module_info
+{
+ const char *name;
+ const char *version;
+} pg_module_info;
+
/* Definition of the magic block structure */
typedef struct
{
@@ -469,10 +475,15 @@ typedef struct
int namedatalen; /* NAMEDATALEN */
int float8byval; /* FLOAT8PASSBYVAL */
char abi_extra[32]; /* see pg_config_manual.h */
+ pg_module_info module_extra;
} Pg_magic_struct;
-/* The actual data block contents */
-#define PG_MODULE_MAGIC_DATA \
+/* The actual data block contents
+ *
+ * Fill the module info part with zeros by default. Zero-length module name
+ * indicates that it is not initialised.
+ */
+#define PG_MODULE_MAGIC_DATA(...) \
{ \
sizeof(Pg_magic_struct), \
PG_VERSION_NUM / 100, \
@@ -481,6 +492,7 @@ typedef struct
NAMEDATALEN, \
FLOAT8PASSBYVAL, \
FMGR_ABI_EXTRA, \
+ {__VA_ARGS__}, \
}
StaticAssertDecl(sizeof(FMGR_ABI_EXTRA) <= sizeof(((Pg_magic_struct *)
0)->abi_extra),
@@ -500,11 +512,21 @@ 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
+#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/test/modules/test_dsa/expected/test_dsa.out
b/src/test/modules/test_dsa/expected/test_dsa.out
index 266010e77fe..942bf616b5b 100644
--- a/src/test/modules/test_dsa/expected/test_dsa.out
+++ b/src/test/modules/test_dsa/expected/test_dsa.out
@@ -11,3 +11,11 @@ SELECT test_dsa_resowners();
(1 row)
+-- test empty module info
+SELECT module_name,version
+FROM pg_get_modules() WHERE libname LIKE '%test_dsa%';
+ module_name | version
+-------------+---------
+ |
+(1 row)
+
diff --git a/src/test/modules/test_dsa/sql/test_dsa.sql
b/src/test/modules/test_dsa/sql/test_dsa.sql
index c3d8db94372..55d982df45f 100644
--- a/src/test/modules/test_dsa/sql/test_dsa.sql
+++ b/src/test/modules/test_dsa/sql/test_dsa.sql
@@ -2,3 +2,7 @@ CREATE EXTENSION test_dsa;
SELECT test_dsa_basic();
SELECT test_dsa_resowners();
+
+-- test empty module info
+SELECT module_name,version
+FROM pg_get_modules() WHERE libname LIKE '%test_dsa%';
diff --git a/src/test/modules/test_shm_mq/expected/test_shm_mq.out
b/src/test/modules/test_shm_mq/expected/test_shm_mq.out
index c4858b0c205..45d1587ce9b 100644
--- a/src/test/modules/test_shm_mq/expected/test_shm_mq.out
+++ b/src/test/modules/test_shm_mq/expected/test_shm_mq.out
@@ -34,3 +34,11 @@ SELECT test_shm_mq_pipelined(16384, (select
string_agg(chr(32+(random()*95)::int
(1 row)
+-- Check module version
+SELECT module_name, version
+FROM pg_get_modules() WHERE libname LIKE '%test_shm_mq%';
+ module_name | version
+-------------+---------
+ test_shm_mq | 1.0.0
+(1 row)
+
diff --git a/src/test/modules/test_shm_mq/sql/test_shm_mq.sql
b/src/test/modules/test_shm_mq/sql/test_shm_mq.sql
index 9de19d304a2..9576e9f6483 100644
--- a/src/test/modules/test_shm_mq/sql/test_shm_mq.sql
+++ b/src/test/modules/test_shm_mq/sql/test_shm_mq.sql
@@ -10,3 +10,7 @@ SELECT test_shm_mq(1024, 'a', 2001, 1);
SELECT test_shm_mq(32768, (select string_agg(chr(32+(random()*95)::int), '')
from generate_series(1,(100+900*random())::int)), 10000, 1);
SELECT test_shm_mq(100, (select string_agg(chr(32+(random()*95)::int), '')
from generate_series(1,(100+200*random())::int)), 10000, 1);
SELECT test_shm_mq_pipelined(16384, (select
string_agg(chr(32+(random()*95)::int), '') from generate_series(1,270000)),
200, 3);
+
+-- Check module version
+SELECT module_name, version
+FROM pg_get_modules() WHERE libname LIKE '%test_shm_mq%';
diff --git a/src/test/modules/test_shm_mq/test.c
b/src/test/modules/test_shm_mq/test.c
index 443281addd0..130d1f81300 100644
--- a/src/test/modules/test_shm_mq/test.c
+++ b/src/test/modules/test_shm_mq/test.c
@@ -20,7 +20,10 @@
#include "test_shm_mq.h"
-PG_MODULE_MAGIC;
+PG_MODULE_MAGIC_EXT(
+ .name = "test_shm_mq",
+ .version = "1.0.0"
+);
PG_FUNCTION_INFO_V1(test_shm_mq);
PG_FUNCTION_INFO_V1(test_shm_mq_pipelined);
diff --git a/src/test/modules/test_slru/expected/test_slru.out
b/src/test/modules/test_slru/expected/test_slru.out
index 185c56e5d62..65ed2916f8f 100644
--- a/src/test/modules/test_slru/expected/test_slru.out
+++ b/src/test/modules/test_slru/expected/test_slru.out
@@ -267,4 +267,20 @@ SELECT test_slru_page_exists(0x1234500000030);
f
(1 row)
+-- Check module version in case it was loaded on startup
+SELECT module_name, version
+FROM pg_get_modules() WHERE libname LIKE '%test_slru%';
+ module_name | version
+-------------+---------
+ test_slru | 1.2.3
+(1 row)
+
DROP EXTENSION test_slru;
+-- Module still exists and returns the same value after DROP EXTENSION
+SELECT module_name, version
+FROM pg_get_modules() WHERE libname LIKE '%test_slru%';
+ module_name | version
+-------------+---------
+ test_slru | 1.2.3
+(1 row)
+
diff --git a/src/test/modules/test_slru/sql/test_slru.sql
b/src/test/modules/test_slru/sql/test_slru.sql
index b1b376581ab..11064ad9201 100644
--- a/src/test/modules/test_slru/sql/test_slru.sql
+++ b/src/test/modules/test_slru/sql/test_slru.sql
@@ -73,4 +73,12 @@ SELECT test_slru_page_exists(0x1234500000000);
SELECT test_slru_page_exists(0x1234500000020);
SELECT test_slru_page_exists(0x1234500000030);
+-- Check module version in case it was loaded on startup
+SELECT module_name, version
+FROM pg_get_modules() WHERE libname LIKE '%test_slru%';
+
DROP EXTENSION test_slru;
+
+-- Module still exists and returns the same value after DROP EXTENSION
+SELECT module_name, version
+FROM pg_get_modules() WHERE libname LIKE '%test_slru%';
diff --git a/src/test/modules/test_slru/test_slru.c
b/src/test/modules/test_slru/test_slru.c
index 3ea5ceb8552..ed19ee1e719 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -22,7 +22,10 @@
#include "storage/shmem.h"
#include "utils/builtins.h"
-PG_MODULE_MAGIC;
+PG_MODULE_MAGIC_EXT(
+ .name = "test_slru",
+ .version = "1.2.3"
+);
/*
* SQL-callable entry points
--
2.48.1
