public inbox for [email protected]
help / color / mirror / Atom feedFrom: David E. Wheeler <[email protected]>
To: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: RFC: Additional Directory for Extensions
Date: Tue, 25 Jun 2024 18:31:46 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+TgmoYcAmhnSUHKdJsS1wOR-9+y6dbLhfJ3Ad+iZEDmA1mPXg@mail.gmail.com>
References: <CA+TgmoaMk==vOUmWwJ5ZA-EueNqa9RQaWMEiHCF1ZKW_YCcj8g@mail.gmail.com>
<[email protected]>
<CA+TgmoYcAmhnSUHKdJsS1wOR-9+y6dbLhfJ3Ad+iZEDmA1mPXg@mail.gmail.com>
On Jun 25, 2024, at 10:43 AM, Robert Haas <[email protected]> wrote:
> If we want to work on making the sorts of changes that
> you're proposing, let's do it on a separate thread. It's not going to
> be meaningfully harder to move in that direction after some patch like
> this than it is today.
I appreciate this separation of concerns, Robert.
In other news, here’s an updated patch that expands the documentation to record that the destination directory is a prefix, and full paths should be used under it. Also take the opportunity to document the PGXS DESTDIR variable as the thing to use to install files under the destination directory.
It still requires a server restart; I can change it back to superuser-only if that’s the consensus.
For those who prefer a GitHub patch review experience, see this PR:
https://github.com/theory/postgres/pull/3/files
Best,
David
Attachments:
[application/octet-stream] v4-0001-Add-extension_destdir-GUC.patch (11.5K, ../[email protected]/2-v4-0001-Add-extension_destdir-GUC.patch)
download | inline diff:
From 698bdd970c18585320b222ecc2b0906ace3177c6 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <[email protected]>
Date: Tue, 25 Jun 2024 18:28:27 -0400
Subject: [PATCH v4] Add extension_destdir GUC
Based on a [patch] by Christophe Berg in the Debian Project, add a new
GUC, `extension_destdir`, that prepends a directory prefix for extension
loading. This directory is prepended to the `SHAREDIR` paths when
loading extensions (control and SQL files), and to the `$libdir`
directive when loading modules that back functions. Changing the
configuration requires a server restart, and is visible only to super
users.
Also document the PGXS `DESTDIR` variable, which should be used to
install extensions into the proper destination directory.
[patch]: https://salsa.debian.org/postgresql/postgresql/-/blob/17/debian/patches/extension_destdir?ref_type=heads
---
doc/src/sgml/config.sgml | 35 ++++++++
doc/src/sgml/extend.sgml | 12 ++-
src/backend/commands/extension.c | 90 +++++++++++++++++++
src/backend/utils/fmgr/dfmgr.c | 29 +++++-
src/backend/utils/misc/guc_tables.c | 12 +++
src/backend/utils/misc/postgresql.conf.sample | 2 +
src/include/utils/guc.h | 1 +
7 files changed, 179 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0c7a9082c5..350ed5b9c9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10389,6 +10389,41 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
+ <varlistentry id="guc-extension-destdir" xreflabel="extension_destdir">
+ <term><varname>extension_destdir</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>extension_destdir</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies a directory prefix into which extensions should be
+ installed. When set, the postmaster will search this directory for an
+ extension before searching the default paths.
+ </para>
+
+ <para>
+ For example, this configuration:
+<programlisting>
+extension_destdir = '/mnt/extensions'
+</programlisting>
+ will allow <productname>PostgreSQL</productname> to first look for
+ extension control files, SQL files, and loadable modules installed in
+ <literal>/mnt/extensions</literal> and fall back on the
+ default directories if they're not found there.
+ </para>
+
+ <para>
+ Note that the files should be installed in their full paths under the
+ <varname>extension_destdir</varname> prefix. When using
+ <link linkend="extend-pgxs">PGXS</link> to install an extension, pass
+ the destination directory via the <varname>DESTDIR</varname> variable
+ to install the files in the proper location. For more information see
+ <xref linkend="extend-extensions-files-directory"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
</sect1>
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5c..6653955d53 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -669,7 +669,8 @@ RETURNS anycompatible AS ...
<para>
The directory containing the extension's <acronym>SQL</acronym> script
file(s). Unless an absolute path is given, the name is relative to
- the installation's <literal>SHAREDIR</literal> directory. The
+ the <literal>SHAREDIR</literal> under the <xref linkend="guc-extension-destdir"/>
+ prefix and to the installation's <literal>SHAREDIR</literal> directory. The
default behavior is equivalent to specifying
<literal>directory = 'extension'</literal>.
</para>
@@ -1710,6 +1711,15 @@ include $(PGXS)
</listitem>
</varlistentry>
+ <varlistentry id="extend-pgxs-destdir">
+ <term><varname>DESTDIR</varname></term>
+ <listitem>
+ <para>
+ install all files under this directory prefix
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="extend-pgxs-no-installcheck">
<term><varname>NO_INSTALLCHECK</varname></term>
<listitem>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..f3b7735c5b 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -393,6 +393,16 @@ get_extension_control_filename(const char *extname)
get_share_path(my_exec_path, sharepath);
result = (char *) palloc(MAXPGPATH);
+ /*
+ * If extension_destdir is set, try to find the file there first
+ */
+ if (*extension_destdir != '\0')
+ {
+ snprintf(result, MAXPGPATH, "%s%s/extension/%s.control",
+ extension_destdir, sharepath, extname);
+ if (pg_file_exists(result))
+ return result;
+ }
snprintf(result, MAXPGPATH, "%s/extension/%s.control",
sharepath, extname);
@@ -432,6 +442,16 @@ get_extension_aux_control_filename(ExtensionControlFile *control,
scriptdir = get_extension_script_directory(control);
result = (char *) palloc(MAXPGPATH);
+ /*
+ * If extension_destdir is set, try to find the file there first
+ */
+ if (*extension_destdir != '\0')
+ {
+ snprintf(result, MAXPGPATH, "%s%s/%s--%s.control",
+ extension_destdir, scriptdir, control->name, version);
+ if (pg_file_exists(result))
+ return result;
+ }
snprintf(result, MAXPGPATH, "%s/%s--%s.control",
scriptdir, control->name, version);
@@ -450,6 +470,23 @@ get_extension_script_filename(ExtensionControlFile *control,
scriptdir = get_extension_script_directory(control);
result = (char *) palloc(MAXPGPATH);
+ /*
+ * If extension_destdir is set, try to find the file there first
+ */
+ if (*extension_destdir != '\0')
+ {
+ if (from_version)
+ snprintf(result, MAXPGPATH, "%s%s/%s--%s--%s.sql",
+ extension_destdir, scriptdir, control->name, from_version, version);
+ else
+ snprintf(result, MAXPGPATH, "%s%s/%s--%s.sql",
+ extension_destdir, scriptdir, control->name, version);
+ if (pg_file_exists(result))
+ {
+ pfree(scriptdir);
+ return result;
+ }
+ }
if (from_version)
snprintf(result, MAXPGPATH, "%s/%s--%s--%s.sql",
scriptdir, control->name, from_version, version);
@@ -1209,6 +1246,59 @@ get_ext_ver_list(ExtensionControlFile *control)
DIR *dir;
struct dirent *de;
+ /*
+ * If extension_destdir is set, try to find the files there first
+ */
+ if (*extension_destdir != '\0')
+ {
+ char location[MAXPGPATH];
+
+ snprintf(location, MAXPGPATH, "%s%s", extension_destdir,
+ get_extension_script_directory(control));
+ dir = AllocateDir(location);
+ while ((de = ReadDir(dir, location)) != NULL)
+ {
+ char *vername;
+ char *vername2;
+ ExtensionVersionInfo *evi;
+ ExtensionVersionInfo *evi2;
+
+ /* must be a .sql file ... */
+ if (!is_extension_script_filename(de->d_name))
+ continue;
+
+ /* ... matching extension name followed by separator */
+ if (strncmp(de->d_name, control->name, extnamelen) != 0 ||
+ de->d_name[extnamelen] != '-' ||
+ de->d_name[extnamelen + 1] != '-')
+ continue;
+
+ /* extract version name(s) from 'extname--something.sql' filename */
+ vername = pstrdup(de->d_name + extnamelen + 2);
+ *strrchr(vername, '.') = '\0';
+ vername2 = strstr(vername, "--");
+ if (!vername2)
+ {
+ /* It's an install, not update, script; record its version name */
+ evi = get_ext_ver_info(vername, &evi_list);
+ evi->installable = true;
+ continue;
+ }
+ *vername2 = '\0'; /* terminate first version */
+ vername2 += 2; /* and point to second */
+
+ /* if there's a third --, it's bogus, ignore it */
+ if (strstr(vername2, "--"))
+ continue;
+
+ /* Create ExtensionVersionInfos and link them together */
+ evi = get_ext_ver_info(vername, &evi_list);
+ evi2 = get_ext_ver_info(vername2, &evi_list);
+ evi->reachable = lappend(evi->reachable, evi2);
+ }
+ FreeDir(dir);
+ }
+
location = get_extension_script_directory(control);
dir = AllocateDir(location);
while ((de = ReadDir(dir, location)) != NULL)
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index 092004dcf3..25971b25b6 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -35,6 +35,7 @@
#include "miscadmin.h"
#include "storage/fd.h"
#include "storage/shmem.h"
+#include "utils/guc.h"
#include "utils/hsearch.h"
@@ -415,7 +416,7 @@ expand_dynamic_library_name(const char *name)
{
bool have_slash;
char *new;
- char *full;
+ char *full, *full2;
Assert(name);
@@ -430,6 +431,19 @@ expand_dynamic_library_name(const char *name)
else
{
full = substitute_libpath_macro(name);
+ /*
+ * If extension_destdir is set, try to find the file there first
+ */
+ if (*extension_destdir != '\0')
+ {
+ full2 = psprintf("%s%s", extension_destdir, full);
+ if (pg_file_exists(full2))
+ {
+ pfree(full);
+ return full2;
+ }
+ pfree(full2);
+ }
if (pg_file_exists(full))
return full;
pfree(full);
@@ -448,6 +462,19 @@ expand_dynamic_library_name(const char *name)
{
full = substitute_libpath_macro(new);
pfree(new);
+ /*
+ * If extension_destdir is set, try to find the file there first
+ */
+ if (*extension_destdir != '\0')
+ {
+ full2 = psprintf("%s%s", extension_destdir, full);
+ if (pg_file_exists(full2))
+ {
+ pfree(full);
+ return full2;
+ }
+ pfree(full2);
+ }
if (pg_file_exists(full))
return full;
pfree(full);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 46c258be28..8a3c6e2968 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -541,6 +541,7 @@ char *ConfigFileName;
char *HbaFileName;
char *IdentFileName;
char *external_pid_file;
+char *extension_destdir;
char *application_name;
@@ -4487,6 +4488,17 @@ struct config_string ConfigureNamesString[] =
check_canonical_path, NULL, NULL
},
+ {
+ {"extension_destdir", PGC_POSTMASTER, FILE_LOCATIONS,
+ gettext_noop("Path to prepend for extension loading."),
+ gettext_noop("This directory is prepended to paths when loading extensions (control and SQL files), and to the '$libdir' directive when loading modules that back functions. The location is made configurable to allow build-time testing of extensions that do not have been installed to their proper location yet."),
+ GUC_SUPERUSER_ONLY
+ },
+ &extension_destdir,
+ "",
+ NULL, NULL, NULL
+ },
+
{
{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the name of the SSL library."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e0567de219..93a70487b6 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -771,6 +771,8 @@
# - Other Defaults -
#dynamic_library_path = '$libdir'
+#extension_destdir = '' # prepend path when loading extensions
+ # and shared objects (added by Debian)
#gin_fuzzy_search_limit = 0
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index ff506bf48d..eaf0b4f337 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -277,6 +277,7 @@ extern PGDLLIMPORT char *ConfigFileName;
extern PGDLLIMPORT char *HbaFileName;
extern PGDLLIMPORT char *IdentFileName;
extern PGDLLIMPORT char *external_pid_file;
+extern PGDLLIMPORT char *extension_destdir;
extern PGDLLIMPORT char *application_name;
--
2.45.2
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: RFC: Additional Directory for Extensions
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox