On 12/12/24 08:36, Tom Lane wrote:
Andrei Lepikhov <lepi...@gmail.com> writes:
It makes sense. But I want to clarify that I avoided changing
PG_MODULE_MAGIC because the newly introduced structure has a totally
different purpose and usage logic: the struct is designed to check
compatibility, but module info isn't connected to the core version at
all: a single version of the code may be built for multiple PG versions.
At the same time, various versions of the same library may be usable
with the same core.
Surely. But I don't see a need for two separately-looked-up
physical structures. Seems to me it's sufficient to put the
ABI-checking fields into a sub-struct within the magic block.
Okay, I've rewritten the patch to understand how it works. It seems to
work pretty well. I added separate fields for minor and major versions.
--
regards, Andrei Lepikhov
From 0bc431ce0ababc9fe5492773205525a6839f1264 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 v1] Introduce PG_MODULE_MAGIC_EXT macro.
This macro allows dynamically loaded shared libraries (modules) a standard way
to incorporate version (major and minor) and name data. The introduced
catalogue routine module_info 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
---
contrib/auto_explain/auto_explain.c | 2 +-
contrib/auto_explain/t/001_auto_explain.pl | 9 +++
contrib/pg_prewarm/t/001_basic.pl | 8 ++-
.../pg_stat_statements/pg_stat_statements.c | 2 +-
src/backend/utils/fmgr/dfmgr.c | 72 ++++++++++++++++++-
src/include/catalog/pg_proc.dat | 6 ++
src/include/fmgr.h | 24 ++++++-
7 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index f2eaa8e494..f482ab125d 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -20,7 +20,7 @@
#include "executor/instrument.h"
#include "utils/guc.h"
-PG_MODULE_MAGIC;
+PG_MODULE_MAGIC_EXT("auto_explain", 1, 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 0e5b34afa9..5db7df5478 100644
--- a/contrib/auto_explain/t/001_auto_explain.pl
+++ b/contrib/auto_explain/t/001_auto_explain.pl
@@ -53,6 +53,15 @@ like(
qr/Seq Scan on pg_class/,
"sequential scan logged, text mode");
+$log_contents = $node->safe_psql(
+ "postgres", q{SELECT substring(libname, 'auto_explain'),module_name,major_ver,minor_ver
+ FROM module_info() WHERE major_ver IS NOT NULL;});
+
+like(
+ $log_contents,
+ qr/auto_explain|auto_explain|1|0/,
+ "Check module version");
+
# Prepared query.
$log_contents = query_log($node,
q{PREPARE get_proc(name) AS SELECT * FROM pg_proc WHERE proname = $1; EXECUTE get_proc('int4pl');}
diff --git a/contrib/pg_prewarm/t/001_basic.pl b/contrib/pg_prewarm/t/001_basic.pl
index 825d3448ee..f0197e33af 100644
--- a/contrib/pg_prewarm/t/001_basic.pl
+++ b/contrib/pg_prewarm/t/001_basic.pl
@@ -25,8 +25,14 @@ $node->safe_psql("postgres",
. "CREATE TABLE test(c1 int);\n"
. "INSERT INTO test SELECT generate_series(1, 100);");
-# test read mode
+# test empty module info
my $result =
+ $node->safe_psql("postgres", "SELECT substring(libname, 'pg_prewarm'),
+ module_name,major_ver,minor_ver FROM module_info();");
+like($result, qr/pg_prewarm|||/, 'Return null if module does not provide an info');
+
+# test read mode
+$result =
$node->safe_psql("postgres", "SELECT pg_prewarm('test', 'read');");
like($result, qr/^[1-9][0-9]*$/, 'read mode succeeded');
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 602cae54ff..24374e9209 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -71,7 +71,7 @@
#include "utils/memutils.h"
#include "utils/timestamp.h"
-PG_MODULE_MAGIC;
+PG_MODULE_MAGIC_EXT("pg_stat_statements", 1, 12);
/* Location of permanent stats file (valid when database is shut down) */
#define PGSS_DUMP_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pg_stat_statements.stat"
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index 8e81ecc749..e11f9d20a6 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,9 @@ typedef struct df_files
ino_t inode; /* Inode number of file */
#endif
void *handle; /* a handle for pg_dl* functions */
+ char name[NAMEDATALEN];
+ int32 major_ver;
+ int32 minor_ver;
char filename[FLEXIBLE_ARRAY_MEMBER]; /* Full pathname of file */
} DynamicFileList;
@@ -75,7 +80,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});
/*
@@ -246,7 +251,8 @@ 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)
+ 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 +264,20 @@ internal_load_library(const char *libname)
/* issue suitable complaint */
incompatible_module_error(libname, &module_magic_data);
}
+
+ if (magic_data_ptr->module_extra.name[0] != '\0')
+ {
+ strcpy(file_scanner->name, magic_data_ptr->module_extra.name);
+ file_scanner->major_ver = magic_data_ptr->module_extra.major_ver;
+ file_scanner->minor_ver = magic_data_ptr->module_extra.minor_ver;
+ }
+ else
+ {
+ /*
+ * No need additional efforts. Zero-filled data serves as is in
+ * the 'not initialized' state
+ */
+ }
}
else
{
@@ -675,3 +695,51 @@ RestoreLibraryState(char *start_address)
start_address += strlen(start_address) + 1;
}
}
+
+Datum
+module_info(PG_FUNCTION_ARGS)
+{
+ FuncCallContext *funcctx;
+ MemoryContext oldcontext;
+ DynamicFileList *file_scanner = NULL;
+ TupleDesc tupdesc;
+ Datum result;
+ Datum values[4];
+ bool isnull[4] = {0,0,0,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->name[0] == '\0')
+ {
+ isnull[0] = isnull[1] = isnull[2] = true;
+ }
+ else
+ {
+ values[0] = CStringGetTextDatum(file_scanner->name);
+ values[1] = Int32GetDatum(file_scanner->major_ver);
+ values[2] = Int32GetDatum(file_scanner->minor_ver);
+ }
+
+ values[3] = 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 0f22c21723..8638b5842c 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 => 'module_info', provolatile => 's', prorettype => 'record',
+ proretset => 't', proargtypes => '', proallargtypes => '{text,int4,int4,text}',
+ proargmodes => '{o,o,o,o}', prorows => 10,
+ proargnames => '{module_name,major_ver,minor_ver,libname}',
+ prosrc => 'module_info' },
{ 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 1e3795de4a..de1ce3401f 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -459,6 +459,13 @@ extern PGDLLEXPORT void _PG_init(void);
*-------------------------------------------------------------------------
*/
+typedef struct pg_module_info
+{
+ char name[NAMEDATALEN];
+ int32 major_ver;
+ int32 minor_ver;
+} pg_module_info;
+
/* Definition of the magic block structure */
typedef struct
{
@@ -469,10 +476,12 @@ 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 \
+/* 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 +490,7 @@ typedef struct
NAMEDATALEN, \
FLOAT8PASSBYVAL, \
FMGR_ABI_EXTRA, \
+ {__VA_ARGS__}, \
}
StaticAssertDecl(sizeof(FMGR_ABI_EXTRA) <= sizeof(((Pg_magic_struct *) 0)->abi_extra),
@@ -500,11 +510,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
--
2.39.5
