On 27.08.25 14:39, Matheus Alcantara wrote:
> On Tue Aug 26, 2025 at 3:26 AM -03, Dilip Kumar wrote:
>> On Tue, Aug 26, 2025 at 11:31 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>>
>>> On Sun, Aug 24, 2025 at 5:59 PM Srinath Reddy Sadipiralla
>>> <srinath2133@gmail.com> wrote:
>>>>
>>>> Hi,
>>>> Thanks Dilip and Matheus for working on this , i reviewed the
>>>> latest patch given my Matheus and it LGTM but i have doubt that in
>>>> f777d773878 commit the $libdir was moved out from
>>>> expand_dynamic_library_name into load_external_function because if
>>>> someone specifies LOAD '$libdir/foo' explicitly they want to get
>>>> the foo.so from $libdir not from other paths given in
>>>> dynamic_library_path ,i think same should go for the case when we
>>>> do "create extension" will try to execute the sql script which will
>>>> replace the MODULE_PATHNAME with module_pathname from .control file
>>>> lets say which is $libdir/foo ,now during the sql functions
>>>> execution this calls the load_external_function where we strip
>>>> $libdir and we are going to load the foo.so from other paths
>>>> specified in dynamic_library_path, isn't that a problem , i think
>>>> this case is also same as with the LOAD.
>>>
>>> IMHO the cases like $libdir/foo is not a problem because first we will
>>> strip the $libdir and then we will try to find the foo in the
>>> 'dynamic_library_path' and the default value of dynamic_library_path
>>> is $libdir so everything should work fine. OTOH the case I report has
>>> $libdir/xyz/foo, in this case it doesn't search in
>>> 'dynamic_library_path' instead try to replace the $libdir macro, but
>>> we have already stripped the $libdir so this will not behave
>>> correctly, so if we have any extra slash in path we need to retain the
>>> $libdir
>>
>> Please find the revised patch with improved comments and commit messages.
>>
> Thanks for the improvements! LGTM.
committed, thanks