From f8647720dae41449ca6dd6d10d0fe5317b272de3 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Wed, 23 Apr 2025 16:11:24 -0300 Subject: [PATCH] Make "directory" work with extension control path Previously extensions installed on a custom path that is available via extension_control_path GUC that set the "directory" field on .control file was not being able to CREATE. This was happening because on get_extension_script_directory was hard coded to search for the script files only on the share system dir. This commit fix this issue by using the control->control_dir as a share dir to return the path of the extension script files. --- doc/src/sgml/config.sgml | 22 ++-- src/backend/commands/extension.c | 102 +++++++++++++----- .../t/001_extension_control_path.pl | 93 +++++++++++----- 3 files changed, 161 insertions(+), 56 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 14661ac2cc637..2a55be2342c12 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -11029,16 +11029,26 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' (Use pg_config --sharedir to find out the name of this directory.) For example: -extension_control_path = '/usr/local/share/postgresql/extension:/home/my_project/share/extension:$system' +extension_control_path = '/usr/local/share/postgresql:/home/my_project/share:$system' or, in a Windows environment: -extension_control_path = 'C:\tools\postgresql\extension;H:\my_project\share\extension;$system' +extension_control_path = 'C:\tools\postgresql;H:\my_project\share;$system' - Note that the path elements should typically end in - extension if the normal installation layouts are - followed. (The value for $system already includes - the extension suffix.) + Note that all specified path elements are expected to have a + extension subdirectory which will have the .control + and .sql files, this path suffix is automatically appended at the end + of each path. (The value for $system already + includes the extension subdirectory.) + + + + Also note that extension .control file may configure the .sql files to + be placed in another directory using the directory + field (See for details.). If + the configured directory is a relative path, it will search based on the + path that the .control file was found, for example, + /home/my_project/share/<bla>. diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 180f4af9be36a..577b9f2ff0db8 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -83,6 +83,8 @@ Oid CurrentExtensionObject = InvalidOid; typedef struct ExtensionControlFile { char *name; /* name of the extension */ + char *basedir; /* base directory where control and script + * files are located */ char *control_dir; /* directory where control file was found */ char *directory; /* directory for script files */ char *default_version; /* default install target version, if any */ @@ -153,6 +155,7 @@ static void ExecAlterExtensionContentsRecurse(AlterExtensionContentsStmt *stmt, static char *read_whole_file(const char *filename, int *length); static ExtensionControlFile *new_ExtensionControlFile(const char *extname); +char *find_in_paths(const char *basename, List *paths); /* * get_extension_oid - given an extension name, look up the OID @@ -374,8 +377,15 @@ get_extension_control_directories(void) piece = palloc(len + 1); strlcpy(piece, ecp, len + 1); - /* Substitute the path macro if needed */ - mangled = substitute_path_macro(piece, "$system", system_dir); + /* + * Substitute the path macro if needed or append "extension" + * suffix in case is a custom extension control path. + */ + if (strcmp(piece, "$system") == 0) + mangled = substitute_path_macro(piece, "$system", system_dir); + else + mangled = psprintf("%s/extension", piece); + pfree(piece); /* Canonicalize the path based on the OS and add to the list */ @@ -401,28 +411,16 @@ get_extension_control_directories(void) static char * find_extension_control_filename(ExtensionControlFile *control) { - char sharepath[MAXPGPATH]; - char *system_dir; char *basename; - char *ecp; char *result; + List *paths; Assert(control->name); - get_share_path(my_exec_path, sharepath); - system_dir = psprintf("%s/extension", sharepath); - basename = psprintf("%s.control", control->name); - /* - * find_in_path() does nothing if the path value is empty. This is the - * historical behavior for dynamic_library_path, but it makes no sense for - * extensions. So in that case, substitute a default value. - */ - ecp = Extension_control_path; - if (strlen(ecp) == 0) - ecp = "$system"; - result = find_in_path(basename, ecp, "extension_control_path", "$system", system_dir); + paths = get_extension_control_directories(); + result = find_in_paths(basename, paths); if (result) { @@ -439,12 +437,11 @@ find_extension_control_filename(ExtensionControlFile *control) static char * get_extension_script_directory(ExtensionControlFile *control) { - char sharepath[MAXPGPATH]; - char *result; - /* * The directory parameter can be omitted, absolute, or relative to the - * installation's share directory. + * installation's base directory, which can be the sharedir or a custom + * path that it was set extension_control_path. It depends where the + * .control file was found. */ if (!control->directory) return pstrdup(control->control_dir); @@ -452,11 +449,8 @@ get_extension_script_directory(ExtensionControlFile *control) if (is_absolute_path(control->directory)) return pstrdup(control->directory); - get_share_path(my_exec_path, sharepath); - result = (char *) palloc(MAXPGPATH); - snprintf(result, MAXPGPATH, "%s/%s", sharepath, control->directory); - - return result; + Assert(control->basedir != NULL); + return psprintf("%s/%s", control->basedir, control->directory); } static char * @@ -550,6 +544,14 @@ parse_extension_control_file(ExtensionControlFile *control, errhint("The extension must first be installed on the system where PostgreSQL is running."))); } + /* Assert that the control_dir ends with /extension */ + Assert(control->control_dir != NULL); + Assert(strcmp(control->control_dir + strlen(control->control_dir) - strlen("/extension"), "/extension") == 0); + + control->basedir = pnstrdup( + control->control_dir, + strlen(control->control_dir) - strlen("/extension")); + if ((file = AllocateFile(filename, "r")) == NULL) { /* no complaint for missing auxiliary file */ @@ -3863,3 +3865,51 @@ new_ExtensionControlFile(const char *extname) return control; } + + +/* + * Work in a very similar way with find_in_path but it receives an already + * parsed List of paths to search the basename and it do not support macro + * replacement or custom error messages (for simplicity). + * + * By "already parsed List of paths" this function expected that paths already + * have all macros replaced. + */ +char * +find_in_paths(const char *basename, List *paths) +{ + ListCell *cell; + + /* + * If the paths variable is empty, don't do a path search. + */ + if (paths == NIL) + return NULL; + + foreach(cell, paths) + { + char *path = (char *) lfirst(cell); + char *full; + + Assert(path != NULL); + + canonicalize_path(path); + + /* only absolute paths */ + if (!is_absolute_path(path)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("component in parameter \"extension_control_path\" is not an absolute path"))); + + full = psprintf("%s/%s", path, basename); + + elog(DEBUG3, "%s: trying \"%s\"", __func__, full); + + if (pg_file_exists(full)) + return full; + + pfree(full); + } + + return NULL; +} diff --git a/src/test/modules/test_extensions/t/001_extension_control_path.pl b/src/test/modules/test_extensions/t/001_extension_control_path.pl index c186c1470f71f..1ef79d7574fd5 100644 --- a/src/test/modules/test_extensions/t/001_extension_control_path.pl +++ b/src/test/modules/test_extensions/t/001_extension_control_path.pl @@ -5,6 +5,7 @@ use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +use File::Path qw( make_path ); my $node = PostgreSQL::Test::Cluster->new('node'); @@ -12,25 +13,14 @@ # Create a temporary directory for the extension control file my $ext_dir = PostgreSQL::Test::Utils::tempdir(); +make_path("$ext_dir/extension"); + my $ext_name = "test_custom_ext_paths"; -my $control_file = "$ext_dir/$ext_name.control"; -my $sql_file = "$ext_dir/$ext_name--1.0.sql"; - -# Create .control .sql file -open my $cf, '>', $control_file or die "Could not create control file: $!"; -print $cf "comment = 'Test extension_control_path'\n"; -print $cf "default_version = '1.0'\n"; -print $cf "relocatable = true\n"; -close $cf; - -# Create --1.0.sql file -open my $sqlf, '>', $sql_file or die "Could not create sql file: $!"; -print $sqlf "/* $sql_file */\n"; -print $sqlf - "-- complain if script is sourced in psql, rather than via CREATE EXTENSION\n"; -print $sqlf - qq'\\echo Use "CREATE EXTENSION $ext_name" to load this file. \\quit\n'; -close $sqlf; +create_extension($ext_name, $ext_dir); + +my $ext_name2 = "test_custom_ext_paths_using_directory"; +make_path("$ext_dir/$ext_name2"); +create_extension($ext_name2, $ext_dir, $ext_name2); # Use the correct separator and escape \ when running on Windows. my $sep = $windows_os ? ";" : ":"; @@ -48,6 +38,7 @@ "custom extension control directory path configured"); $node->safe_psql('postgres', "CREATE EXTENSION $ext_name"); +$node->safe_psql('postgres', "CREATE EXTENSION $ext_name2"); my $ret = $node->safe_psql('postgres', "select * from pg_available_extensions where name = '$ext_name'"); @@ -55,26 +46,80 @@ "test_custom_ext_paths|1.0|1.0|Test extension_control_path", "extension is installed correctly on pg_available_extensions"); -my $ret2 = $node->safe_psql('postgres', +$ret = $node->safe_psql('postgres', "select * from pg_available_extension_versions where name = '$ext_name'"); -is( $ret2, +is( $ret, "test_custom_ext_paths|1.0|t|t|f|t|||Test extension_control_path", "extension is installed correctly on pg_available_extension_versions"); +$ret = $node->safe_psql('postgres', + "select * from pg_available_extensions where name = '$ext_name2'"); +is( $ret, + "test_custom_ext_paths_using_directory|1.0|1.0|Test extension_control_path", + "extension is installed correctly on pg_available_extensions"); + +$ret = $node->safe_psql('postgres', + "select * from pg_available_extension_versions where name = '$ext_name2'"); +is( $ret, + "test_custom_ext_paths_using_directory|1.0|t|t|f|t|||Test extension_control_path", + "extension is installed correctly on pg_available_extension_versions"); + # Ensure that extensions installed on $system is still visible when using with # custom extension control path. -my $ret3 = $node->safe_psql('postgres', +$ret = $node->safe_psql('postgres', "select count(*) > 0 as ok from pg_available_extensions where name = 'plpgsql'" ); -is($ret3, "t", +is($ret, "t", "\$system extension is installed correctly on pg_available_extensions"); -my $ret4 = $node->safe_psql('postgres', +$ret = $node->safe_psql('postgres', "set extension_control_path = ''; select count(*) > 0 as ok from pg_available_extensions where name = 'plpgsql'" ); -is($ret4, "t", +is($ret, "t", "\$system extension is installed correctly on pg_available_extensions with empty extension_control_path" ); +# Test with an extension that does not exists +my ($code, $stdout, $stderr) = $node->psql('postgres', "CREATE EXTENSION invalid"); +is($code, 3, 'error to create an extension that does not exists'); +like($stderr, qr/ERROR: extension "invalid" is not available/); + +sub create_extension +{ + my ($ext_name, $ext_dir, $directory) = @_; + + my $control_file = "$ext_dir/extension/$ext_name.control"; + my $sql_file; + + if (defined $directory) + { + $sql_file = "$ext_dir/$directory/$ext_name--1.0.sql"; + } + else + { + $sql_file = "$ext_dir/extension/$ext_name--1.0.sql"; + } + + # Create .control .sql file + open my $cf, '>', $control_file or die "Could not create control file: $!"; + print $cf "comment = 'Test extension_control_path'\n"; + print $cf "default_version = '1.0'\n"; + print $cf "relocatable = true\n"; + if (defined $directory) + { + print $cf "directory = $directory"; + } + close $cf; + + # Create --1.0.sql file + open my $sqlf, '>', $sql_file or die "Could not create sql file: $!"; + print $sqlf "/* $sql_file */\n"; + print $sqlf + "-- complain if script is sourced in psql, rather than via CREATE EXTENSION\n"; + print $sqlf + qq'\\echo Use "CREATE EXTENSION $ext_name" to load this file. \\quit\n'; + close $sqlf; +} + done_testing();