Re: RFC: Additional Directory for Extensions

On Apr 3, 2024, at 12:46 PM, Christoph Berg <myon(at)debian(dot)org> wrote:

> Thanks for bringing this up, I should have submitted this years ago.
> (The patch is originally from September 2020.)

That’s okay, it’s still 2020 in some ways. 😂

> I designed the patch to require a superuser to set it, so it doesn't
> matter very much by which mechanism it gets updated. There should be
> little reason to vary it at run-time, so I'd be fine with requiring a
> restart, but otoh, why restrict the superuser from reloading it if
> they know what they are doing?

I think that’s fair. I’ll keep it requiring a restart now, on the theory it would be easier to loosen it later than have to tighten it later.

> I'm not sure the concept of "core libraries" exists. PG happens to
> dlopen things at run time, and it doesn't know/care if they were
> installed by users or by the original PG server. Also, an exploited
> libpgtypes library is not worse than any other untrusted "user"
> library, so you really don't want to allow users to provide their own
> .so files, no matter by what mechanism.

Yes, I guess my concern is whether it could be used to “shadow” core libraries. Maybe it’s no different, really.

>> This would also allow the lookup of extension libraries prefixed by the directory field from the control file, which would enable much tidier extension installation: The control file, SQL scripts, and DSOs could all be in a single directory for an extension.
>
> Nice idea, but that would mean installing .so files into PGSHAREDIR.
> Perhaps the whole extension stuff would have to move to PKGLIBDIR
> instead.

Yes, I was just poking around the code, and realized that, when extension functions are created they may or may not not use `MODULE_PATHNAME`, but in any event, there is nothing different about loading an extension DSO than any other DSO. I was hoping to find a path where it knows it’s opening a DSO for the purpose of an extension, so we could limit the lookup there. But that does not (currently) exist.

Maybe we could add an `$extensiondir` variable to complement `$libdir`?

Or is PGKLIBDIR is the way to go? I’m not familiar with it. It looks like extension JIT files are put there already.

> Fwiw, I wrote this patch to solve the problem of testing extensions at
> build-time where the build process does not have write access to
> PGSHAREDIR. It solves that problem quite well, almost all PG
> extensions have build-time test coverage now (where there was
> basically 0 before).

Yeah, good additional use case.

On Apr 3, 2024, at 1:03 PM, Christoph Berg <myon(at)debian(dot)org> wrote:

> Also, it's called extension "destdir" because it behaves like DESTDIR
> in Makefiles: It prepends the given path to the path that PG is trying
> to open when set. So it doesn't allow arbitrary new locations as of
> now, just /home/build/foo-1/debian/foo/usr/share/postgresql/17/extension
> in addition to /usr/share/postgresql/17/extension. (That is what the
> Debian package build process needs, so that restriction/design choice
> made sense.

Right, this makes perfect sense, in that you don’t have to copy all the extension files from the destdir to the SHAREDIR to test them, which I imagine could be a PITA.

> That's also included in the current GUC description:
>
> This directory is prepended to paths when loading extensions
> (control and SQL files), and to the '$libdir' directive when
> loading modules that back functions. The location is made
> configurable to allow build-time testing of extensions that do not
> have been installed to their proper location yet.
>
> Perhaps I should have included a more verbose "NOT FOR PRODUCTION"
> there.

The use cases I described upthread are very much production use cases. Do you think it’s not for production just because we need to really think it through?

I’ve added some docs based on your GUC description; updated patch attached.

Best,

David