Subject: Re: RFC: Additional Directory for Extensions

From: Robert Haas <[email-masked]>
Date: Tue, 25 Jun 2024 10:43:52 -0400

On Tue, Jun 25, 2024 at 6:12 AM Alvaro Herrera <[email-masked]> wrote:
> Now, I'm not saying that this is an easy journey.  But if we don't
> start, we're not going to get there.

I actually kind of agree with you. I think I've said something similar
in a previous email to the list somewhere. But I don't agree that this
patch should be burdened with taking the first step. We seem to often
find reasons why patches that packagers for prominent distributions
are carrying shouldn't be put into core, and I think that's a bad
habit. They're not going to stop applying those packages because we
refuse to put suitable functionality in core; we're just creating a
situation where lots of people are running slightly patched versions
of PostgreSQL instead of straight-up PostgreSQL. That's not improving
anything. If we want to work on making the sorts of changes that
you're proposing, let's do it on a separate thread. It's not going to
be meaningfully harder to move in that direction after some patch like
this than it is today.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: David E. Wheeler <[email-masked]>
Date: Tue, 25 Jun 2024 18:31:46 -0400

On Jun 25, 2024, at 10:43 AM, Robert Haas <[email-masked]> wrote:

> If we want to work on making the sorts of changes that

I appreciate this separation of concerns, Robert.

In other news, here’s an updated patch that expands the documentation to record that the destination directory is a prefix, and full paths should be used under it. Also take the opportunity to document the PGXS DESTDIR variable as the thing to use to install files under the destination directory.

It still requires a server restart; I can change it back to superuser-only if that’s the consensus.

For those who prefer a GitHub patch review experience, see this PR:

  https://github.com/theory/postgres/pull/3/files

Best,

David

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Wed, 26 Jun 2024 11:30:39 +0200

> In other news, here’s an updated patch that expands the documentation to record that the destination directory is a prefix, and full paths should be used under it. Also take the opportunity to docu…

Docs are much clearer now thanks.

        full = substitute_libpath_macro(name);
+       /*
+        * If extension_destdir is set, try to find the file there first
+        */
+       if (*extension_destdir != '\0')
+       {
+           full2 = psprintf("%s%s", extension_destdir, full);
+           if (pg_file_exists(full2))
+           {
+               pfree(full);
+               return full2;
+           }
+           pfree(full2);
+       }

I think this should be done differently. For two reasons:
1. I don't think extension_destdir should be searched when $libdir is
not part of the name.
2. find_in_dynamic_libpath currently doesn't use extension_destdir at
all, so if there is no slash in the filename we do not search
extension_destdir.

I feel like changing the substitute_libpath_macro function a bit (or
adding a new similar function) is probably the best way to achieve
that.

We should also check somewhere (probably GUC check hook) that
extension_destdir is an absolute path.

> It still requires a server restart;

When reading the code I see no reason why this cannot be PGC_SIGHUP.
Even though it's probably not needed to change on a running server, I
think it's better to allow that. Even just so people can disable it if
necessary for some reason without restarting the process.

> I can change it back to superuser-only if that’s the consensus.

It still is GUC_SUPERUSER_ONLY, right?

>   https://github.com/theory/postgres/pull/3/files

Sidenote: The "D" link for each patch on cfbot[1] now gives a similar
link for all commitfest entries[2].

[1]: http://cfbot.cputube.org/
[2]: https://github.com/postgresql-cfbot/postgresql/compare/cf/4913~1...cf/4913

---

From: "David E. Wheeler" <[email-masked]>
Date: Mon, 8 Jul 2024 12:01:57 -0400

>  https://github.com/theory/postgres/pull/3/files

Rebased and restored PGC_SUSET in the attached v5 patch, plus noted the required privileges in the docs.

Best,

David

---

From: Gabriele Bartolini <[email-masked]>
Date: Wed, 21 Aug 2024 21:59:46 +0200

Hi everyone,

Apologies for only starting to look into this now. Thanks, David, for
pushing this forward.

I want to emphasize the importance of this patch for the broader adoption
of extensions in immutable container environments, such as those used by
the CloudNativePG operator in Kubernetes.

To provide some context, one of the key principles of CloudNativePG is that
containers, once started, cannot be modified—this includes the installation
of Postgres extensions and their libraries. This restriction prevents us
from adding extensions on the fly, requiring them to be included in the
main PostgreSQL operand image. As a result, users who need specific
extensions must build custom images through automated pipelines (see:
https://cloudnative-pg.io/blog/creating-container-images/).

We’ve been considering ways to improve this process for some time. The
direction we're exploring involves mounting an ephemeral volume that
contains the necessary extensions (namely $sharedir and $pkglibdir from
pg_config). These volumes would be created and populated with the required
extensions when the container starts and destroyed when it shuts down. To
make this work, each extension must be independently packaged as a
container image containing the appropriate files for a specific extension
version, tailored to the architecture, distribution, OS version, and
Postgres version.

I’m committed to thoroughly reviewing this patch, testing it with
CloudNativePG and a few extensions, and providing feedback as soon as
possible.

Best,
Gabriele

-- 
Gabriele Bartolini
Vice President, Cloud Native at EDB
enterprisedb.com

---

From: Craig Ringer <[email-masked]>
Date: Thu, 22 Aug 2024 11:07:17 +1200

On Thu, 22 Aug 2024 at 08:00, Gabriele Bartolini
<[email-masked]> wrote:
>
> Hi everyone,
>
> Apologies for only starting to look into this now. Thanks, David, for pushing this forward.

 100%. I've wanted this for some time but never had time to cook up a patch.

> I want to emphasize the importance of this patch for the broader adoption of extensions in immutable container environments, such as those used by the CloudNativePG operator in Kubernetes.

It's also very relevant for local development and testing.

Right now postgres makes it impossible to locally compile and install
an extension for a distro-packaged postgres (whether from upstream
repos or PGDG repos) without dirtying the distro-managed filesystem
subtrees with local changes under /usr etc, because it cannot be
configured to look for locally installed extensions on non-default
paths.

> To provide some context, one of the key principles of CloudNativePG is that containers, once started, cannot be modified—this includes the installation of Postgres extensions and their libraries. This restriction prevents us from adding extensions on the fly, requiring them to be included in the main PostgreSQL operand image. As a result, users who need specific extensions must build custom images through automated pipelines (see: https://cloudnative-pg.io/blog/creating-container-images/).

It may be possible to weaken this restriction somewhat thanks to the
upcoming https://kubernetes.io/blog/2024/08/16/kubernetes-1-31-image-volume-source/
feature that permits additional OCI images to be mounted as read-only
volumes on a workload. This would still only permit mounting at
Pod-creation time, not runtime mounting and unmonuting, but means the
base postgres image could be supplemented by mounting additional
images for extensions.

For example, one might mount image "postgis-vX.Y.Z" image onto base
image "postgresql-16" if support for PostGIS is desired, without then
having to bake every possible extension anyone might ever want into
the base image. This solves all sorts of messy issues with upgrades
and new version releases.

But for it to work, it must be possible to tell postgres to look in
_multiple places_ for extension .sql scripts and control files. This
is presently possible for modules (dynamic libraries, .so / .dylib /
.dll) but without a way to also configure the path for extensions it's
of very limited utility.

> We’ve been considering ways to improve this process for some time. The direction we're exploring involves mounting an ephemeral volume that contains the necessary extensions (namely $sharedir and $pkglibdir from pg_config). These volumes would be created and populated with the required extensions when the container starts and destroyed when it shuts down. To make this work, each extension must be independently packaged as a container image containing the appropriate files for a specific extension version, tailored to the architecture, distribution, OS version, and Postgres version.

Right. And there might be more than one of them.

So IMO this should be a _path_ to search for extension control files
and SQL scripts.

If the current built-in default extension dir was exposed as a var
$extdir like we do for $libdir, this might look something like this
for local development and testing while working with a packaged
postgres build:

    SET extension_search_path = $extsdir, /opt/myapp/extensions,
/usr/local/postgres/my-custom-extension/extensions;
    SET dynamic_library_path = $libdir, /opt/myapp/lib,
/usr/local/postgres/my-custom-extension/lib

or in the container extensions case, something like:

    SET extension_search_path = $extsdir,
/mnt/extensions/pg16/postgis-vX.Y/extensions,
/mnt/extensions/pg16/gosuperfast/extensions;
    SET dynamic_library_path = $libdir,
/mnt/extensions/pg16/postgis-vX.Y/lib,
/mnt/extensions/pg16/gosuperfast/lib;

For safety, it might make sense to impose the restriction that if an
extension control file is found in a given directory, SQL scripts will
also only be looked for in that same directory. That way there's no
chance of accidentally mixing and matching SQL scripts from different
versions of an extension if it appears twice on the extension search
path in different places. The rule for loading SQL scripts would be:

* locate first directory on path contianing matching extension control file
* use this directory as the extension directory for all subsequent SQL
script loading and running actions

--
Craig Ringer
EnterpriseDB

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Thu, 22 Aug 2024 09:31:54 +0200

> /mnt/extensions/pg16/gosuperfast/extensions;

It looks like you want one directory per extension, so that list would
get pretty long if you have multiple extensions. Maybe (as a follow up
change), we should start to support a * as a wildcard in both of these
GUCs. So you could say:

SET extension_search_path = /mnt/extensions/pg16/*

To mean effectively the same as you're describing above.

---

From: Gabriele Bartolini <[email-masked]>
Date: Thu, 22 Aug 2024 10:58:45 +0200

Hi Craig,

> It's also very relevant for local development and testing.

Yep, which is the original goal of Christoph IIRC.

> upcoming
> https://kubernetes.io/blog/2024/08/16/kubernetes-1-31-image-volume-source/

I'm really excited about that feature, but it's still in the alpha stage.
However, I don't anticipate any issues for the future general availability
(GA) release. Regardless, we may need to consider a temporary solution that
is compatible with existing Kubernetes and possibly Postgres versions (but
that's beyond the purpose of this thread).

> and new version releases.

Yep.

> .dll) but without a way to also configure the path for extensions it's

Agree.

> So IMO this should be a _path_ to search for extension control files

I like this. I also prefer the name `extension_search_path`.

> script loading and running actions

It could work, but it requires some prototyping and exploration. I'm
willing to participate and use CloudNativePG as a test bed.

Cheers,
Gabriele

---

From: Gabriele Bartolini <[email-masked]>
Date: Thu, 22 Aug 2024 10:59:47 +0200

Hi Jelte,

> To mean effectively the same as you're describing above.

That'd be great. +1.

---

From: Craig Ringer <[email-masked]>
Date: Fri, 23 Aug 2024 10:14:20 +1200

>> SET extension_search_path = /mnt/extensions/pg16/*

Agreed, that'd be handy, but not worth blocking the underlying capability for.

Except possibly to the degree that the feature should reserve wildcard
characters and require them to be escaped if they appear on a path, so
there's no BC break if it's added later.

---

From: Craig Ringer <[email-masked]>
Date: Fri, 23 Aug 2024 10:16:00 +1200

> there's no BC break if it's added later.

... though on second thoughts, it might make more sense to just
recursively search directories found under each path entry. Rules like
'search if a redundant trailing / is present' can be an option.

That way there's no icky path escaping needed for normal configuration.