Hi,
Thanks for reporting this!
On Fri Aug 22, 2025 at 7:34 AM -03, Dilip Kumar wrote:
> Simple test to reproduce:
>
> Change the module path for any extension as below, like I have done
> for amcheck and then copy the .so file at $libdir/test/ folder.
> module_pathname = '$libdir/test/amcheck'
>
> While creating the extension it fail to load the file because, after
> commit f777d773878 we remove the $libdir from the path[1] and later in
> expand_dynamic_library_name() since there is a '/' in remaining path
> we will call full = substitute_path_macro(name, "$libdir",
> pkglib_path); to generate the full path by substituting the "$libdir"
> macro[2]. But we have already removed $libdir from the filename, so
> there will be no substitution and it will just search the
> 'test/amcheck' path, which was not intended.
>
> IMHO the fix should be that we need to preserve the original name as
> well, and in the else case we should pass the original name which is
> '$libdir/test/amcheck' in this case so that macro substitution can
> work properly?
>
Using multiple names seems a bit confusing to me TBH. I think that a
more simple approach would be strip the $libdir from filename on
load_external_function() only if after the strip it doesn't have any
remaining path separator. With this, if the user write $libdir/foo/bar
we assume that he wants to load from a chield directory on $libidir, so
we don't strip from filename, otherwise if module_pathname only has
$libdir/bar we assume that it is using the previous behaviour and we can
strip to enable the search path on extension_control_path GUC.
Please see the attached patch to check if it solves your issue? I
still need to perform some other tests to validate that this fix is
fully correct but the check-world is passing.
Thoughts?
--
Matheus Alcantara
From c635b700b229a94101461e034bf739451c9f3b67 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
Date: Fri, 22 Aug 2025 10:23:31 -0300
Subject: [PATCH v0] Don't strip $libdir from nested module_pathnames
---
src/backend/utils/fmgr/dfmgr.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index 4bb84ff7087..2ba86f3aeba 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -100,12 +100,20 @@ load_external_function(const char *filename, const char *funcname,
void *retval;
/*
- * If the value starts with "$libdir/", strip that. This is because many
- * extensions have hardcoded '$libdir/foo' as their library name, which
- * prevents using the path.
+ * Check if the filename starts with "$libdir/". If it does, and the
+ * remaining part of the string contains no directory separators, advance
+ * the pointer to strip the prefix. This ensures that only simple
+ * filenames (e.g., "$libdir/foo") are stripped, while full paths (e.g.,
+ * "$libdir/foo/bar") are left untouched.
+ *
+ * This is because many extensions have hardcoded '$libdir/foo' as their
+ * library name, which prevents using the search path.
*/
if (strncmp(filename, "$libdir/", 8) == 0)
- filename += 8;
+ {
+ if (first_dir_separator(filename + 8) == NULL)
+ filename += 8;
+ }
/* Expand the possibly-abbreviated filename to an exact path name */
fullname = expand_dynamic_library_name(filename);
--
2.39.5 (Apple Git-154)
