Re: Potential problem in commit f777d773878 and 4f7f7b03758

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