Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Don't strip $libdir from LOAD command

  1. BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    PG Bug reporting form <noreply@postgresql.org> — 2025-05-08T19:55:51Z

    The following bug has been logged on the website:

Bug reference:      18920
Logged by:          Evan Si
Email address:      evsi@amazon.com
PostgreSQL version: 18beta1
Operating system:   Amazon Linux 2
Description:        

Hi,
The 18beta1 documentation for the LOAD command has remained the same since
previous major versions.
https://www.postgresql.org/docs/18/sql-load.html
In the postgres installation, I can copy something e.g. auto_explain into a
plugins subdirectory of `pg_config --pkglibdir`
```
cp $(pg_config --pkglibdir)/auto_explain.so $(pg_config
--pkglibdir)/plugins/auto_explain.so
```
then from the server start:
```
psql (18beta1)
Type "help" for help.
postgres=# LOAD '$libdir/plugins/auto_explain.so';
ERROR:  could not access file "plugins/auto_explain.so": No such file or
directory
postgres=# \q
```
This behavior differs from documentation and previous majors. It seems to me
that $libdir is getting stripped out from this commit:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=4f7f7b0
--
Evan

  2. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-06-02T14:05:12Z

    Hi,

Thanks for reporting this!

On 08/05/25 16:55, PG Bug reporting form wrote:
> Hi,
> The 18beta1 documentation for the LOAD command has remained the same since
> previous major versions.
> https://www.postgresql.org/docs/18/sql-load.html
> In the postgres installation, I can copy something e.g. auto_explain into a
> plugins subdirectory of `pg_config --pkglibdir`
> ```
> cp $(pg_config --pkglibdir)/auto_explain.so $(pg_config
> --pkglibdir)/plugins/auto_explain.so
> ```
> then from the server start:
> ```
> psql (18beta1)
> Type "help" for help.
> postgres=# LOAD '$libdir/plugins/auto_explain.so';
> ERROR:  could not access file "plugins/auto_explain.so": No such file or
> directory
> postgres=# \q
> ```
> This behavior differs from documentation and previous majors. It seems to me
> that $libdir is getting stripped out from this commit:
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=4f7f7b0
>

The problem is how we are sripping the $libdir from module_pathname. I
think that we should still strip the $libdir from module_pathname when
expanding from a .control file, but we should not expand via LOAD
command, since the user may explicitly specify the $libdir prefix as it
is the case here.

Having this logic on expand_dynamic_library_name() force us to srip the
$libdir for all scenarios, so I think that a better place to have this
is on load_external_function() that is called when executing a SQL
script from an extension. Another approach would be to change the
signature of expand_dynamic_library_name() to receive a boolean and only
srip the $libdir if it's true, but I think that the expand function
should not handle the strip logic, it should only expand the dynamic
library path and nothing else.

I'm also attaching a second patch that has a TAP test case that
reproduce this issue, it has some hacks to move dynamic library files
around to exercice the problem so I'm not sure if it could be good to
also include these tests on the final patch.

-- 
Matheus Alcantara

  3. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-02T17:16:59Z

    On Mon, Jun 02, 2025 at 11:05:12AM -0300, Matheus Alcantara wrote:
> The problem is how we are sripping the $libdir from module_pathname. I
> think that we should still strip the $libdir from module_pathname when
> expanding from a .control file, but we should not expand via LOAD
> command, since the user may explicitly specify the $libdir prefix as it
> is the case here.
> 
> Having this logic on expand_dynamic_library_name() force us to srip the
> $libdir for all scenarios, so I think that a better place to have this
> is on load_external_function() that is called when executing a SQL
> script from an extension.

Thanks for the patch.  This seems generally reasonable to me.

-- 
nathan

  4. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Rahila Syed <rahilasyed90@gmail.com> — 2025-06-02T19:56:34Z

    Hi,

Thank you both for the patch and the explanation regarding the fix.


> Another approach would be to change the
> signature of expand_dynamic_library_name() to receive a boolean and only
> srip the $libdir if it's true, but I think that the expand function
> should not handle the strip logic, it should only expand the dynamic
> library path and nothing else.
>

The reasoning makes sense to me.


> I'm also attaching a second patch that has a TAP test case that
> reproduce this issue, it has some hacks to move dynamic library files
> around to exercice the problem so I'm not sure if it could be good to
> also include these tests on the final patch.
>

The TAP test is useful in testing various paths of extension files.
It will be cleaner if the dummy_index_am.so  is deleted at the end of the
tests from the $libdir.

Currently, the tests seem to be failing as follows for windows:

[19:07:13.107] #   Failed test 'create extension from custom extension
control path'[19:07:13.107] #   at
C:/cirrus/src/test/modules/test_extensions/t/002_strip_modulepathname.pl
line 102.[19:07:13.107] #          got: '2'[19:07:13.107] #
expected: '0'[19:07:13.107] # Looks like you failed 1 test of
7.[19:07:13.107] [19:07:13.107] (test program exited with status code
1)

Details here:  Windows - Server 2019, VS 2019 - Meson & ninja - Cirrus CI
<https://cirrus-ci.com/task/5241991606304768>

Thank you,
Rahila Syed

  5. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-06-02T20:53:23Z

    On 02/06/25 16:56, Rahila Syed wrote:
>> I'm also attaching a second patch that has a TAP test case that
>> reproduce this issue, it has some hacks to move dynamic library files
>> around to exercice the problem so I'm not sure if it could be good to
>> also include these tests on the final patch.
>>
>
> The TAP test is useful in testing various paths of extension files.
> It will be cleaner if the dummy_index_am.so  is deleted at the end of the
> tests from the $libdir.
>
I think that the problem is that some other tests may use this
extension, so deleting at the end may cause other tests that use it to
fail depending on the other that is executed? I actually delete the .so
in the middle of the test but I copy it back to $libdir at the end to
avoid this possible side effect.

> Currently, the tests seem to be failing as follows for windows:
>
> [19:07:13.107] #   Failed test 'create extension from custom extension
> control path'[19:07:13.107] #   at
> C:/cirrus/src/test/modules/test_extensions/t/002_strip_modulepathname.pl
> line 102.[19:07:13.107] #          got: '2'[19:07:13.107] #
> expected: '0'[19:07:13.107] # Looks like you failed 1 test of
> 7.[19:07:13.107] [19:07:13.107] (test program exited with status code
> 1)
>
> Details here:  Windows - Server 2019, VS 2019 - Meson & ninja - Cirrus CI
> <https://cirrus-ci.com/task/5241991606304768>
>
Thanks, I'll take a look

-- 
Matheus Alcantara

  6. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-06-02T22:56:01Z

    On 02/06/25 16:56, Rahila Syed wrote:
> Currently, the tests seem to be failing as follows for windows:
>
> [19:07:13.107] #   Failed test 'create extension from custom extension
> control path'[19:07:13.107] #   at
> C:/cirrus/src/test/modules/test_extensions/t/002_strip_modulepathname.pl
> line 102.[19:07:13.107] #          got: '2'[19:07:13.107] #
> expected: '0'[19:07:13.107] # Looks like you failed 1 test of
> 7.[19:07:13.107] [19:07:13.107] (test program exited with status code
> 1)
>
> Details here:  Windows - Server 2019, VS 2019 - Meson & ninja - Cirrus CI
> <https://cirrus-ci.com/task/5241991606304768>
>

Fixed, v2 attached. The 0001 patch remains the same as v1.

-- 
Matheus Alcantara

  7. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Michael Paquier <michael@paquier.xyz> — 2025-06-03T05:48:06Z

    On Mon, Jun 02, 2025 at 07:56:01PM -0300, Matheus Alcantara wrote:
> On 02/06/25 16:56, Rahila Syed wrote:
>> Currently, the tests seem to be failing as follows for windows:
>>
>> [19:07:13.107] #   Failed test 'create extension from custom extension
>> control path'[19:07:13.107] #   at
>> C:/cirrus/src/test/modules/test_extensions/t/002_strip_modulepathname.pl
>> line 102.[19:07:13.107] #          got: '2'[19:07:13.107] #
>> expected: '0'[19:07:13.107] # Looks like you failed 1 test of
>> 7.[19:07:13.107] [19:07:13.107] (test program exited with status code
>> 1)
>>
>> Details here:  Windows - Server 2019, VS 2019 - Meson & ninja - Cirrus CI
>> <https://cirrus-ci.com/task/5241991606304768>
> 
> Fixed, v2 attached. The 0001 patch remains the same as v1.

I am adding an open item for Peter Eisentraut, for 4f7f7b037585.  The
patch looks sensible at quick glance, I did not check it in details
regarding the internals of dfmgr.c.

I don't like much the tests you are adding here.  First, the
cross-platform requirements to copy a library to the plugins/ folder
is annoying.  The actual issue is that we don't have an installation
rule to be able to install a library to a plugins/ path to allow
non-superusers to load it, no?  I mean, as 4f7f7b037585 states, we
have a `make install prefix=/else/where` but the TAP tests cannot
shape a temporary installation with it.  If we had something like that
for meson and makefiles, we could then reuse EXTRA_INSTALL to force a
library to exist where we want it to be, for the sake of testing
coverage.

I am not completely sure that the tests are completely waterproof,
either.  Some distributions like fancy installation folder layers,
like Debian, and such things have proven to break these folk's tests.
Having a centralized rule could be also useful for out-of-core
extensions, to give these a way to install something inside a plugins/
folder.  At least that may be better than requiring pg_config to get
the basic library install path.

Second, requiring dummy_index_am inside the tests test_extensions is
adding unwelcome complexity across the test modules.
--
Michael

  8. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-06-03T12:31:17Z

    On 03/06/25 02:48, Michael Paquier wrote:
> I don't like much the tests you are adding here.  First, the
> cross-platform requirements to copy a library to the plugins/ folder
> is annoying.  The actual issue is that we don't have an installation
> rule to be able to install a library to a plugins/ path to allow
> non-superusers to load it, no?  I mean, as 4f7f7b037585 states, we
> have a `make install prefix=/else/where` but the TAP tests cannot
> shape a temporary installation with it.  If we had something like that
> for meson and makefiles, we could then reuse EXTRA_INSTALL to force a
> library to exist where we want it to be, for the sake of testing
> coverage.
>
> I am not completely sure that the tests are completely waterproof,
> either.  Some distributions like fancy installation folder layers,
> like Debian, and such things have proven to break these folk's tests.
> Having a centralized rule could be also useful for out-of-core
> extensions, to give these a way to install something inside a plugins/
> folder.  At least that may be better than requiring pg_config to get
> the basic library install path.
>
> Second, requiring dummy_index_am inside the tests test_extensions is
> adding unwelcome complexity across the test modules.
>

I also don't like these tests, it has a lot of hacks as I've mentioned in
my first email and I'm almost sure that if we push this it will not work
on a lot of build farm animals. I just wrote these tests to make it
possible to reproduce the issue on my local machine and make the fix
easier, and I just shared here so that folks can also have a way to
reproduce the issue and maybe share other ideas to test this.

-- 
Matheus Alcantara

  9. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Michael Paquier <michael@paquier.xyz> — 2025-06-04T00:44:03Z

    On Tue, Jun 03, 2025 at 09:31:17AM -0300, Matheus Alcantara wrote:
> I just wrote these tests to make it
> possible to reproduce the issue on my local machine and make the fix
> easier, and I just shared here so that folks can also have a way to
> reproduce the issue and maybe share other ideas to test this.

+1.  These are useful to check the patch, there's no denying that.  My
apologies if my previous message was confusing.
--
Michael

  10. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Peter Eisentraut <peter@eisentraut.org> — 2025-06-04T09:42:52Z

    On 04.06.25 02:44, Michael Paquier wrote:
> On Tue, Jun 03, 2025 at 09:31:17AM -0300, Matheus Alcantara wrote:
>> I just wrote these tests to make it
>> possible to reproduce the issue on my local machine and make the fix
>> easier, and I just shared here so that folks can also have a way to
>> reproduce the issue and maybe share other ideas to test this.
> 
> +1.  These are useful to check the patch, there's no denying that.  My
> apologies if my previous message was confusing.

I have committed the patch without the additional tests, as discussed.

  11. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-06-04T11:31:40Z

    On Tue, Jun 3, 2025 at 9:44 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Jun 03, 2025 at 09:31:17AM -0300, Matheus Alcantara wrote:
> > I just wrote these tests to make it
> > possible to reproduce the issue on my local machine and make the fix
> > easier, and I just shared here so that folks can also have a way to
> > reproduce the issue and maybe share other ideas to test this.
>
> +1.  These are useful to check the patch, there's no denying that.  My
> apologies if my previous message was confusing.
>
No worries, thanks very much for the comments and taking time to review!

-- 
Matheus Alcantara

  12. Re: BUG #18920: LOAD '$libdir/plugins' no longer works in 18beta1

    Matheus Alcantara <matheusssilv97@gmail.com> — 2025-06-04T11:32:15Z

    On Wed, Jun 4, 2025 at 6:42 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 04.06.25 02:44, Michael Paquier wrote:
> > On Tue, Jun 03, 2025 at 09:31:17AM -0300, Matheus Alcantara wrote:
> >> I just wrote these tests to make it
> >> possible to reproduce the issue on my local machine and make the fix
> >> easier, and I just shared here so that folks can also have a way to
> >> reproduce the issue and maybe share other ideas to test this.
> >
> > +1.  These are useful to check the patch, there's no denying that.  My
> > apologies if my previous message was confusing.
>
> I have committed the patch without the additional tests, as discussed.
>

Thanks!

-- 
Matheus Alcantara