public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: David E. Wheeler <[email protected]>
To: Craig Ringer <[email protected]>
Cc: Gabriele Bartolini <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: RFC: Additional Directory for Extensions
Date: Mon, 11 Nov 2024 08:16:05 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+TgmoaMk==vOUmWwJ5ZA-EueNqa9RQaWMEiHCF1ZKW_YCcj8g@mail.gmail.com>
	<[email protected]>
	<CA+TgmoYcAmhnSUHKdJsS1wOR-9+y6dbLhfJ3Ad+iZEDmA1mPXg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CA+VUV5oWkMVvtNB0BG2CM8SrmOkeW6fEtXGehbniHqR9MhMzVw@mail.gmail.com>
	<CAGRY4nxP6A5Dz23g+aGD-agdVwUj_qrG6szd-mWc0E5OFMBg4w@mail.gmail.com>
	<[email protected]>
	<CAGRY4ny=Oy-F0qCRFQa=XtEqyUbMygvbqqKahAvbae7v4LTOrQ@mail.gmail.com>
	<[email protected]>
	<CAGRY4nx8Fmea4udk--PKdvfgpNdqC+rcueAmGwuw2PD9dv+zqQ@mail.gmail.com>
	<[email protected]>

I implemented a patch along the lines Craig had suggested.  It's a new 
GUC variable that is a path for extension control files.  It's called 
extension_control_path, and it works exactly the same way as 
dynamic_library_path.  Except that the magic token is called $system 
instead of $libdir.  In fact, most of the patch is refactoring the 
routines in dfmgr.c to not hardcode dynamic_library_path but allow 
searching for any file in any path.  Once a control file is found, the 
other extension support files (script files and auxiliary control files) 
are looked for in the same directory.

This works pretty much fine for the use cases that have been presented 
here, including installing extensions outside of the core installation 
tree (for CNPG and Postgres.app) and for testing uninstalled extensions 
(for Debian).

There are some TODOs in the patch.  Some of those are for documentation 
that needs to be completed.  Others are for functions like 
pg_available_extensions() that need to be rewritten to be aware of the 
path.  I think this would be pretty straightforward.

Some open problems or discussion points:

- You can install extensions into alternative directories using PGXS like

     make install datadir=/else/where/share pkglibdir=/else/where/lib

This works.  I was hoping it would work to use

     make install prefix=/else/where

but that doesn't because of some details in Makefile.global.  I think we 
can tweak that a little bit to make that work too.

- With the current patch, if you install into datadir=/else/where/share, 
then you need to set extension_control_path=/else/where/share/extension. 
  This is a bit confusing.  Maybe we want to make the "extension" part 
implicit.

- The biggest problem is that many extensions set in their control file

     module_pathname = '$libdir/foo'

This disables the use of dynamic_library_path, so this whole idea of 
installing an extension elsewhere won't work that way.  The obvious 
solution is that extensions change this to just 'foo'.  But this will 
require a lot updating work for many extensions, or a lot of patching by 
packagers.

Maybe we could devise some sort of rule that if the extension control 
file is found via the path outside of $system, then the leading $libdir 
is ignored.

From 08620bae551bb6b2e3a44aa3df4c8751ad06f804 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 11 Nov 2024 07:29:47 +0100
Subject: [PATCH v0] extension_control_path

The new GUC extension_control_path specifies a path to look for
extension control files.  The default value is $system, which looks in
the compiled-in location, as before.

The path search uses the same code and works in the same way as
dynamic_library_path.

Discussion: https://www.postgresql.org/message-id/flat/[email protected]
---
 doc/src/sgml/config.sgml                      | 13 ++++
 doc/src/sgml/extend.sgml                      |  1 +
 doc/src/sgml/ref/create_extension.sgml        |  1 +
 src/backend/commands/extension.c              | 56 ++++++++++++++--
 src/backend/utils/fmgr/dfmgr.c                | 64 +++++++++++--------
 src/backend/utils/misc/guc_tables.c           | 12 ++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/commands/extension.h              |  2 +
 src/include/fmgr.h                            |  2 +
 9 files changed, 118 insertions(+), 34 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d54f9049569..c4790de7102 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10434,6 +10434,19 @@ <title>Other Defaults</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-extension-control-path" xreflabel="extension_control_path">
+      <term><varname>extension_control_path</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>extension_control_path</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        TODO
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-gin-fuzzy-search-limit" xreflabel="gin_fuzzy_search_limit">
       <term><varname>gin_fuzzy_search_limit</varname> (<type>integer</type>)
       <indexterm>
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5ce..77a517d5167 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -635,6 +635,7 @@ <title>Extension Files</title>
     <primary>control file</primary>
    </indexterm>
 
+    <!-- TODO -->
     <para>
      The <command>CREATE EXTENSION</command> command relies on a control
      file for each extension, which must be named the same as the extension
diff --git a/doc/src/sgml/ref/create_extension.sgml b/doc/src/sgml/ref/create_extension.sgml
index ca2b80d669c..c26218f3adf 100644
--- a/doc/src/sgml/ref/create_extension.sgml
+++ b/doc/src/sgml/ref/create_extension.sgml
@@ -92,6 +92,7 @@ <title>Parameters</title>
         installed. <productname>PostgreSQL</productname> will create the
         extension using details from the file
         <literal>SHAREDIR/extension/</literal><replaceable class="parameter">extension_name</replaceable><literal>.control</literal>.
+        <!-- TODO -->
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index af6bd8ff426..b328e8e28f4 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -69,6 +69,9 @@
 #include "utils/varlena.h"
 
 
+/* GUC */
+char	   *Extension_control_path;
+
 /* Globally visible state variables */
 bool		creating_extension = false;
 Oid			CurrentExtensionObject = InvalidOid;
@@ -79,6 +82,7 @@ Oid			CurrentExtensionObject = InvalidOid;
 typedef struct ExtensionControlFile
 {
 	char	   *name;			/* name of the extension */
+	char	   *control_dir;	/* directory where control file was found */
 	char	   *directory;		/* directory for script files */
 	char	   *default_version;	/* default install target version, if any */
 	char	   *module_pathname;	/* string to substitute for
@@ -328,6 +332,9 @@ is_extension_script_filename(const char *filename)
 	return (extension != NULL) && (strcmp(extension, ".sql") == 0);
 }
 
+// TODO
+// This is now only for finding/listing available extensions.  Rewrite to use
+// path.  See further TODOs below.
 static char *
 get_extension_control_directory(void)
 {
@@ -341,16 +348,36 @@ get_extension_control_directory(void)
 	return result;
 }
 
+/*
+ * Find control file for extension with name in control->name, looking in the
+ * path.  Return the full file name, or NULL if not found.  If found, the
+ * directory is recorded in control->control_dir.
+ */
 static char *
-get_extension_control_filename(const char *extname)
+find_extension_control_filename(ExtensionControlFile *control)
 {
 	char		sharepath[MAXPGPATH];
+	char	   *system_dir;
+	char	   *basename;
 	char	   *result;
 
+	Assert(control->name);
+
 	get_share_path(my_exec_path, sharepath);
-	result = (char *) palloc(MAXPGPATH);
-	snprintf(result, MAXPGPATH, "%s/extension/%s.control",
-			 sharepath, extname);
+	system_dir = psprintf("%s/extension", sharepath);
+
+	basename = psprintf("%s.control", control->name);
+
+	result = find_in_path(basename, Extension_control_path, "extension_control_path", "$system", system_dir);
+
+	if (result)
+	{
+		const char *p;
+
+		p = strrchr(result, '/');
+		Assert(p);
+		control->control_dir = pnstrdup(result, p - result);
+	}
 
 	return result;
 }
@@ -366,7 +393,7 @@ get_extension_script_directory(ExtensionControlFile *control)
 	 * installation's share directory.
 	 */
 	if (!control->directory)
-		return get_extension_control_directory();
+		return pstrdup(control->control_dir);
 
 	if (is_absolute_path(control->directory))
 		return pstrdup(control->directory);
@@ -444,7 +471,15 @@ parse_extension_control_file(ExtensionControlFile *control,
 	if (version)
 		filename = get_extension_aux_control_filename(control, version);
 	else
-		filename = get_extension_control_filename(control->name);
+		filename = find_extension_control_filename(control);
+
+	if (!filename)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), // XXX?
+				 errmsg("extension \"%s\" is not available", control->name),
+				 errhint("The extension must first be installed on the system where PostgreSQL is running.")));
+	}
 
 	if ((file = AllocateFile(filename, "r")) == NULL)
 	{
@@ -457,6 +492,7 @@ parse_extension_control_file(ExtensionControlFile *control,
 				return;
 			}
 
+			// TODO: this check is obsolete?
 			/* missing control file indicates extension is not installed */
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -2114,6 +2150,8 @@ RemoveExtensionById(Oid extId)
  * The system view pg_available_extensions provides a user interface to this
  * SRF, adding information about whether the extensions are installed in the
  * current DB.
+ *
+ * TODO
  */
 Datum
 pg_available_extensions(PG_FUNCTION_ARGS)
@@ -2194,6 +2232,8 @@ pg_available_extensions(PG_FUNCTION_ARGS)
  * The system view pg_available_extension_versions provides a user interface
  * to this SRF, adding information about which versions are installed in the
  * current DB.
+ *
+ * TODO
  */
 Datum
 pg_available_extension_versions(PG_FUNCTION_ARGS)
@@ -2366,6 +2406,8 @@ get_available_versions_for_extension(ExtensionControlFile *pcontrol,
  * directory.  That's not a bulletproof check, since the file might be
  * invalid, but this is only used for hints so it doesn't have to be 100%
  * right.
+ *
+ * TODO
  */
 bool
 extension_file_exists(const char *extensionName)
@@ -2445,6 +2487,8 @@ convert_requires_to_datum(List *requires)
 /*
  * This function reports the version update paths that exist for the
  * specified extension.
+ *
+ * TODO
  */
 Datum
 pg_extension_update_paths(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index c7aa789b51b..d348d30d9e4 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -81,8 +81,7 @@ static void incompatible_module_error(const char *libname,
 									  const Pg_magic_struct *module_magic_data) pg_attribute_noreturn();
 static char *expand_dynamic_library_name(const char *name);
 static void check_restricted_library_name(const char *name);
-static char *substitute_libpath_macro(const char *name);
-static char *find_in_dynamic_libpath(const char *basename);
+static char *substitute_path_macro(const char *str, const char *macro, const char *value);
 
 /* Magic structure that module needs to match to be accepted */
 static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
@@ -408,7 +407,7 @@ incompatible_module_error(const char *libname,
 /*
  * If name contains a slash, check if the file exists, if so return
  * the name.  Else (no slash) try to expand using search path (see
- * find_in_dynamic_libpath below); if that works, return the fully
+ * find_in_path below); if that works, return the fully
  * expanded file name.  If the previous failed, append DLSUFFIX and
  * try again.  If all fails, just return the original name.
  *
@@ -427,13 +426,13 @@ expand_dynamic_library_name(const char *name)
 
 	if (!have_slash)
 	{
-		full = find_in_dynamic_libpath(name);
+		full = find_in_path(name, Dynamic_library_path, "dynamic_library_path", "$libdir", pkglib_path);
 		if (full)
 			return full;
 	}
 	else
 	{
-		full = substitute_libpath_macro(name);
+		full = substitute_path_macro(name, "$libdir", pkglib_path);
 		if (pg_file_exists(full))
 			return full;
 		pfree(full);
@@ -443,14 +442,14 @@ expand_dynamic_library_name(const char *name)
 
 	if (!have_slash)
 	{
-		full = find_in_dynamic_libpath(new);
+		full = find_in_path(new, Dynamic_library_path, "dynamic_library_path", "$libdir", pkglib_path);
 		pfree(new);
 		if (full)
 			return full;
 	}
 	else
 	{
-		full = substitute_libpath_macro(new);
+		full = substitute_path_macro(new, "$libdir", pkglib_path);
 		pfree(new);
 		if (pg_file_exists(full))
 			return full;
@@ -485,47 +484,56 @@ check_restricted_library_name(const char *name)
  * Result is always freshly palloc'd.
  */
 static char *
-substitute_libpath_macro(const char *name)
+substitute_path_macro(const char *str, const char *macro, const char *value)
 {
 	const char *sep_ptr;
 
-	Assert(name != NULL);
+	Assert(str != NULL);
+	Assert(macro[0] == '$');
 
-	/* Currently, we only recognize $libdir at the start of the string */
-	if (name[0] != '$')
-		return pstrdup(name);
+	/* Currently, we only recognize $macro at the start of the string */
+	if (str[0] != '$')
+		return pstrdup(str);
 
-	if ((sep_ptr = first_dir_separator(name)) == NULL)
-		sep_ptr = name + strlen(name);
+	if ((sep_ptr = first_dir_separator(str)) == NULL)
+		sep_ptr = str + strlen(str);
 
-	if (strlen("$libdir") != sep_ptr - name ||
-		strncmp(name, "$libdir", strlen("$libdir")) != 0)
+	if (strlen(macro) != sep_ptr - str ||
+		strncmp(str, macro, strlen(macro)) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_NAME),
-				 errmsg("invalid macro name in dynamic library path: %s",
-						name)));
+				 errmsg("invalid macro name in path: %s",
+						str)));
 
-	return psprintf("%s%s", pkglib_path, sep_ptr);
+	return psprintf("%s%s", value, sep_ptr);
 }
 
 
 /*
  * Search for a file called 'basename' in the colon-separated search
- * path Dynamic_library_path.  If the file is found, the full file name
+ * path given.  If the file is found, the full file name
  * is returned in freshly palloc'd memory.  If the file is not found,
  * return NULL.
+ *
+ * path_param is the name of the parameter that path came from, for error
+ * messages.
+ *
+ * macro and macro_val allow substituting a macro; see
+ * substitute_path_macro().
  */
-static char *
-find_in_dynamic_libpath(const char *basename)
+char *
+find_in_path(const char *basename, const char *path, const char *path_param,
+			 const char *macro, const char *macro_val)
 {
 	const char *p;
 	size_t		baselen;
 
 	Assert(basename != NULL);
 	Assert(first_dir_separator(basename) == NULL);
-	Assert(Dynamic_library_path != NULL);
+	Assert(path != NULL);
+	Assert(path_param != NULL);
 
-	p = Dynamic_library_path;
+	p = path;
 	if (strlen(p) == 0)
 		return NULL;
 
@@ -542,7 +550,7 @@ find_in_dynamic_libpath(const char *basename)
 		if (piece == p)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_NAME),
-					 errmsg("zero-length component in parameter \"dynamic_library_path\"")));
+					 errmsg("zero-length component in parameter \"%s\"", path_param)));
 
 		if (piece == NULL)
 			len = strlen(p);
@@ -552,7 +560,7 @@ find_in_dynamic_libpath(const char *basename)
 		piece = palloc(len + 1);
 		strlcpy(piece, p, len + 1);
 
-		mangled = substitute_libpath_macro(piece);
+		mangled = substitute_path_macro(piece, macro, macro_val);
 		pfree(piece);
 
 		canonicalize_path(mangled);
@@ -561,13 +569,13 @@ find_in_dynamic_libpath(const char *basename)
 		if (!is_absolute_path(mangled))
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_NAME),
-					 errmsg("component in parameter \"dynamic_library_path\" is not an absolute path")));
+					 errmsg("component in parameter \"%s\" is not an absolute path", path_param)));
 
 		full = palloc(strlen(mangled) + 1 + baselen + 1);
 		sprintf(full, "%s/%s", mangled, basename);
 		pfree(mangled);
 
-		elog(DEBUG3, "find_in_dynamic_libpath: trying \"%s\"", full);
+		elog(DEBUG3, "%s: trying \"%s\"", __func__, full);
 
 		if (pg_file_exists(full))
 			return full;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8a67f01200c..5a00b99daba 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -39,6 +39,7 @@
 #include "catalog/namespace.h"
 #include "catalog/storage.h"
 #include "commands/async.h"
+#include "commands/extension.h"
 #include "commands/event_trigger.h"
 #include "commands/tablespace.h"
 #include "commands/trigger.h"
@@ -4233,6 +4234,17 @@ struct config_string ConfigureNamesString[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"extension_control_path", PGC_SUSET, CLIENT_CONN_OTHER,
+			gettext_noop("Sets the path for extension control files."),
+			gettext_noop("TODO"),
+			GUC_SUPERUSER_ONLY
+		},
+		&Extension_control_path,
+		"$system",
+		NULL, NULL, NULL
+	},
+
 	{
 		{"krb_server_keyfile", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Sets the location of the Kerberos server key file."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 39a3ac23127..518c4f40719 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -772,6 +772,7 @@
 # - Other Defaults -
 
 #dynamic_library_path = '$libdir'
+#extension_control_path = '$system'
 #gin_fuzzy_search_limit = 0
 
 
diff --git a/src/include/commands/extension.h b/src/include/commands/extension.h
index c6f3f867eb7..fe8a97570ea 100644
--- a/src/include/commands/extension.h
+++ b/src/include/commands/extension.h
@@ -17,6 +17,8 @@
 #include "catalog/objectaddress.h"
 #include "parser/parse_node.h"
 
+/* GUC */
+extern PGDLLIMPORT char *Extension_control_path;
 
 /*
  * creating_extension is only true while running a CREATE EXTENSION or ALTER
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 1e3795de4a8..2930b61cee5 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -740,6 +740,8 @@ extern bool CheckFunctionValidatorAccess(Oid validatorOid, Oid functionOid);
  */
 extern PGDLLIMPORT char *Dynamic_library_path;
 
+extern char *find_in_path(const char *basename, const char *path, const char *path_param,
+						  const char *macro, const char *macro_val);
 extern void *load_external_function(const char *filename, const char *funcname,
 									bool signalNotFound, void **filehandle);
 extern void *lookup_external_function(void *filehandle, const char *funcname);

base-commit: e7a9496de90657e2161f68b3a5a9b2d9b0b7bb07
-- 
2.47.0



Attachments:

  [text/plain] v0-0001-extension_control_path.patch (15.4K, ../[email protected]/2-v0-0001-extension_control_path.patch)
  download | inline diff:
From 08620bae551bb6b2e3a44aa3df4c8751ad06f804 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 11 Nov 2024 07:29:47 +0100
Subject: [PATCH v0] extension_control_path

The new GUC extension_control_path specifies a path to look for
extension control files.  The default value is $system, which looks in
the compiled-in location, as before.

The path search uses the same code and works in the same way as
dynamic_library_path.

Discussion: https://www.postgresql.org/message-id/flat/[email protected]
---
 doc/src/sgml/config.sgml                      | 13 ++++
 doc/src/sgml/extend.sgml                      |  1 +
 doc/src/sgml/ref/create_extension.sgml        |  1 +
 src/backend/commands/extension.c              | 56 ++++++++++++++--
 src/backend/utils/fmgr/dfmgr.c                | 64 +++++++++++--------
 src/backend/utils/misc/guc_tables.c           | 12 ++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/commands/extension.h              |  2 +
 src/include/fmgr.h                            |  2 +
 9 files changed, 118 insertions(+), 34 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d54f9049569..c4790de7102 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10434,6 +10434,19 @@ <title>Other Defaults</title>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-extension-control-path" xreflabel="extension_control_path">
+      <term><varname>extension_control_path</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>extension_control_path</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        TODO
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-gin-fuzzy-search-limit" xreflabel="gin_fuzzy_search_limit">
       <term><varname>gin_fuzzy_search_limit</varname> (<type>integer</type>)
       <indexterm>
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5ce..77a517d5167 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -635,6 +635,7 @@ <title>Extension Files</title>
     <primary>control file</primary>
    </indexterm>
 
+    <!-- TODO -->
     <para>
      The <command>CREATE EXTENSION</command> command relies on a control
      file for each extension, which must be named the same as the extension
diff --git a/doc/src/sgml/ref/create_extension.sgml b/doc/src/sgml/ref/create_extension.sgml
index ca2b80d669c..c26218f3adf 100644
--- a/doc/src/sgml/ref/create_extension.sgml
+++ b/doc/src/sgml/ref/create_extension.sgml
@@ -92,6 +92,7 @@ <title>Parameters</title>
         installed. <productname>PostgreSQL</productname> will create the
         extension using details from the file
         <literal>SHAREDIR/extension/</literal><replaceable class="parameter">extension_name</replaceable><literal>.control</literal>.
+        <!-- TODO -->
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index af6bd8ff426..b328e8e28f4 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -69,6 +69,9 @@
 #include "utils/varlena.h"
 
 
+/* GUC */
+char	   *Extension_control_path;
+
 /* Globally visible state variables */
 bool		creating_extension = false;
 Oid			CurrentExtensionObject = InvalidOid;
@@ -79,6 +82,7 @@ Oid			CurrentExtensionObject = InvalidOid;
 typedef struct ExtensionControlFile
 {
 	char	   *name;			/* name of the extension */
+	char	   *control_dir;	/* directory where control file was found */
 	char	   *directory;		/* directory for script files */
 	char	   *default_version;	/* default install target version, if any */
 	char	   *module_pathname;	/* string to substitute for
@@ -328,6 +332,9 @@ is_extension_script_filename(const char *filename)
 	return (extension != NULL) && (strcmp(extension, ".sql") == 0);
 }
 
+// TODO
+// This is now only for finding/listing available extensions.  Rewrite to use
+// path.  See further TODOs below.
 static char *
 get_extension_control_directory(void)
 {
@@ -341,16 +348,36 @@ get_extension_control_directory(void)
 	return result;
 }
 
+/*
+ * Find control file for extension with name in control->name, looking in the
+ * path.  Return the full file name, or NULL if not found.  If found, the
+ * directory is recorded in control->control_dir.
+ */
 static char *
-get_extension_control_filename(const char *extname)
+find_extension_control_filename(ExtensionControlFile *control)
 {
 	char		sharepath[MAXPGPATH];
+	char	   *system_dir;
+	char	   *basename;
 	char	   *result;
 
+	Assert(control->name);
+
 	get_share_path(my_exec_path, sharepath);
-	result = (char *) palloc(MAXPGPATH);
-	snprintf(result, MAXPGPATH, "%s/extension/%s.control",
-			 sharepath, extname);
+	system_dir = psprintf("%s/extension", sharepath);
+
+	basename = psprintf("%s.control", control->name);
+
+	result = find_in_path(basename, Extension_control_path, "extension_control_path", "$system", system_dir);
+
+	if (result)
+	{
+		const char *p;
+
+		p = strrchr(result, '/');
+		Assert(p);
+		control->control_dir = pnstrdup(result, p - result);
+	}
 
 	return result;
 }
@@ -366,7 +393,7 @@ get_extension_script_directory(ExtensionControlFile *control)
 	 * installation's share directory.
 	 */
 	if (!control->directory)
-		return get_extension_control_directory();
+		return pstrdup(control->control_dir);
 
 	if (is_absolute_path(control->directory))
 		return pstrdup(control->directory);
@@ -444,7 +471,15 @@ parse_extension_control_file(ExtensionControlFile *control,
 	if (version)
 		filename = get_extension_aux_control_filename(control, version);
 	else
-		filename = get_extension_control_filename(control->name);
+		filename = find_extension_control_filename(control);
+
+	if (!filename)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), // XXX?
+				 errmsg("extension \"%s\" is not available", control->name),
+				 errhint("The extension must first be installed on the system where PostgreSQL is running.")));
+	}
 
 	if ((file = AllocateFile(filename, "r")) == NULL)
 	{
@@ -457,6 +492,7 @@ parse_extension_control_file(ExtensionControlFile *control,
 				return;
 			}
 
+			// TODO: this check is obsolete?
 			/* missing control file indicates extension is not installed */
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -2114,6 +2150,8 @@ RemoveExtensionById(Oid extId)
  * The system view pg_available_extensions provides a user interface to this
  * SRF, adding information about whether the extensions are installed in the
  * current DB.
+ *
+ * TODO
  */
 Datum
 pg_available_extensions(PG_FUNCTION_ARGS)
@@ -2194,6 +2232,8 @@ pg_available_extensions(PG_FUNCTION_ARGS)
  * The system view pg_available_extension_versions provides a user interface
  * to this SRF, adding information about which versions are installed in the
  * current DB.
+ *
+ * TODO
  */
 Datum
 pg_available_extension_versions(PG_FUNCTION_ARGS)
@@ -2366,6 +2406,8 @@ get_available_versions_for_extension(ExtensionControlFile *pcontrol,
  * directory.  That's not a bulletproof check, since the file might be
  * invalid, but this is only used for hints so it doesn't have to be 100%
  * right.
+ *
+ * TODO
  */
 bool
 extension_file_exists(const char *extensionName)
@@ -2445,6 +2487,8 @@ convert_requires_to_datum(List *requires)
 /*
  * This function reports the version update paths that exist for the
  * specified extension.
+ *
+ * TODO
  */
 Datum
 pg_extension_update_paths(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index c7aa789b51b..d348d30d9e4 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -81,8 +81,7 @@ static void incompatible_module_error(const char *libname,
 									  const Pg_magic_struct *module_magic_data) pg_attribute_noreturn();
 static char *expand_dynamic_library_name(const char *name);
 static void check_restricted_library_name(const char *name);
-static char *substitute_libpath_macro(const char *name);
-static char *find_in_dynamic_libpath(const char *basename);
+static char *substitute_path_macro(const char *str, const char *macro, const char *value);
 
 /* Magic structure that module needs to match to be accepted */
 static const Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
@@ -408,7 +407,7 @@ incompatible_module_error(const char *libname,
 /*
  * If name contains a slash, check if the file exists, if so return
  * the name.  Else (no slash) try to expand using search path (see
- * find_in_dynamic_libpath below); if that works, return the fully
+ * find_in_path below); if that works, return the fully
  * expanded file name.  If the previous failed, append DLSUFFIX and
  * try again.  If all fails, just return the original name.
  *
@@ -427,13 +426,13 @@ expand_dynamic_library_name(const char *name)
 
 	if (!have_slash)
 	{
-		full = find_in_dynamic_libpath(name);
+		full = find_in_path(name, Dynamic_library_path, "dynamic_library_path", "$libdir", pkglib_path);
 		if (full)
 			return full;
 	}
 	else
 	{
-		full = substitute_libpath_macro(name);
+		full = substitute_path_macro(name, "$libdir", pkglib_path);
 		if (pg_file_exists(full))
 			return full;
 		pfree(full);
@@ -443,14 +442,14 @@ expand_dynamic_library_name(const char *name)
 
 	if (!have_slash)
 	{
-		full = find_in_dynamic_libpath(new);
+		full = find_in_path(new, Dynamic_library_path, "dynamic_library_path", "$libdir", pkglib_path);
 		pfree(new);
 		if (full)
 			return full;
 	}
 	else
 	{
-		full = substitute_libpath_macro(new);
+		full = substitute_path_macro(new, "$libdir", pkglib_path);
 		pfree(new);
 		if (pg_file_exists(full))
 			return full;
@@ -485,47 +484,56 @@ check_restricted_library_name(const char *name)
  * Result is always freshly palloc'd.
  */
 static char *
-substitute_libpath_macro(const char *name)
+substitute_path_macro(const char *str, const char *macro, const char *value)
 {
 	const char *sep_ptr;
 
-	Assert(name != NULL);
+	Assert(str != NULL);
+	Assert(macro[0] == '$');
 
-	/* Currently, we only recognize $libdir at the start of the string */
-	if (name[0] != '$')
-		return pstrdup(name);
+	/* Currently, we only recognize $macro at the start of the string */
+	if (str[0] != '$')
+		return pstrdup(str);
 
-	if ((sep_ptr = first_dir_separator(name)) == NULL)
-		sep_ptr = name + strlen(name);
+	if ((sep_ptr = first_dir_separator(str)) == NULL)
+		sep_ptr = str + strlen(str);
 
-	if (strlen("$libdir") != sep_ptr - name ||
-		strncmp(name, "$libdir", strlen("$libdir")) != 0)
+	if (strlen(macro) != sep_ptr - str ||
+		strncmp(str, macro, strlen(macro)) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_NAME),
-				 errmsg("invalid macro name in dynamic library path: %s",
-						name)));
+				 errmsg("invalid macro name in path: %s",
+						str)));
 
-	return psprintf("%s%s", pkglib_path, sep_ptr);
+	return psprintf("%s%s", value, sep_ptr);
 }
 
 
 /*
  * Search for a file called 'basename' in the colon-separated search
- * path Dynamic_library_path.  If the file is found, the full file name
+ * path given.  If the file is found, the full file name
  * is returned in freshly palloc'd memory.  If the file is not found,
  * return NULL.
+ *
+ * path_param is the name of the parameter that path came from, for error
+ * messages.
+ *
+ * macro and macro_val allow substituting a macro; see
+ * substitute_path_macro().
  */
-static char *
-find_in_dynamic_libpath(const char *basename)
+char *
+find_in_path(const char *basename, const char *path, const char *path_param,
+			 const char *macro, const char *macro_val)
 {
 	const char *p;
 	size_t		baselen;
 
 	Assert(basename != NULL);
 	Assert(first_dir_separator(basename) == NULL);
-	Assert(Dynamic_library_path != NULL);
+	Assert(path != NULL);
+	Assert(path_param != NULL);
 
-	p = Dynamic_library_path;
+	p = path;
 	if (strlen(p) == 0)
 		return NULL;
 
@@ -542,7 +550,7 @@ find_in_dynamic_libpath(const char *basename)
 		if (piece == p)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_NAME),
-					 errmsg("zero-length component in parameter \"dynamic_library_path\"")));
+					 errmsg("zero-length component in parameter \"%s\"", path_param)));
 
 		if (piece == NULL)
 			len = strlen(p);
@@ -552,7 +560,7 @@ find_in_dynamic_libpath(const char *basename)
 		piece = palloc(len + 1);
 		strlcpy(piece, p, len + 1);
 
-		mangled = substitute_libpath_macro(piece);
+		mangled = substitute_path_macro(piece, macro, macro_val);
 		pfree(piece);
 
 		canonicalize_path(mangled);
@@ -561,13 +569,13 @@ find_in_dynamic_libpath(const char *basename)
 		if (!is_absolute_path(mangled))
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_NAME),
-					 errmsg("component in parameter \"dynamic_library_path\" is not an absolute path")));
+					 errmsg("component in parameter \"%s\" is not an absolute path", path_param)));
 
 		full = palloc(strlen(mangled) + 1 + baselen + 1);
 		sprintf(full, "%s/%s", mangled, basename);
 		pfree(mangled);
 
-		elog(DEBUG3, "find_in_dynamic_libpath: trying \"%s\"", full);
+		elog(DEBUG3, "%s: trying \"%s\"", __func__, full);
 
 		if (pg_file_exists(full))
 			return full;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8a67f01200c..5a00b99daba 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -39,6 +39,7 @@
 #include "catalog/namespace.h"
 #include "catalog/storage.h"
 #include "commands/async.h"
+#include "commands/extension.h"
 #include "commands/event_trigger.h"
 #include "commands/tablespace.h"
 #include "commands/trigger.h"
@@ -4233,6 +4234,17 @@ struct config_string ConfigureNamesString[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"extension_control_path", PGC_SUSET, CLIENT_CONN_OTHER,
+			gettext_noop("Sets the path for extension control files."),
+			gettext_noop("TODO"),
+			GUC_SUPERUSER_ONLY
+		},
+		&Extension_control_path,
+		"$system",
+		NULL, NULL, NULL
+	},
+
 	{
 		{"krb_server_keyfile", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Sets the location of the Kerberos server key file."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 39a3ac23127..518c4f40719 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -772,6 +772,7 @@
 # - Other Defaults -
 
 #dynamic_library_path = '$libdir'
+#extension_control_path = '$system'
 #gin_fuzzy_search_limit = 0
 
 
diff --git a/src/include/commands/extension.h b/src/include/commands/extension.h
index c6f3f867eb7..fe8a97570ea 100644
--- a/src/include/commands/extension.h
+++ b/src/include/commands/extension.h
@@ -17,6 +17,8 @@
 #include "catalog/objectaddress.h"
 #include "parser/parse_node.h"
 
+/* GUC */
+extern PGDLLIMPORT char *Extension_control_path;
 
 /*
  * creating_extension is only true while running a CREATE EXTENSION or ALTER
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 1e3795de4a8..2930b61cee5 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -740,6 +740,8 @@ extern bool CheckFunctionValidatorAccess(Oid validatorOid, Oid functionOid);
  */
 extern PGDLLIMPORT char *Dynamic_library_path;
 
+extern char *find_in_path(const char *basename, const char *path, const char *path_param,
+						  const char *macro, const char *macro_val);
 extern void *load_external_function(const char *filename, const char *funcname,
 									bool signalNotFound, void **filehandle);
 extern void *lookup_external_function(void *filehandle, const char *funcname);

base-commit: e7a9496de90657e2161f68b3a5a9b2d9b0b7bb07
-- 
2.47.0



view thread (44+ 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], [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