Re: RFC: Additional Directory for Extensions

Hello Peter & Co.

On Dec 5, 2024, at 06:07, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:

> I've made a bit of progress on this patch, filled in some documentation and resolved some TODO markers. Also:

Finally getting around to reviewing this patch. Should it be considered part of the previous patch[1] for purposes of commitfest tracking?

I’ve also created a GitHub PR[2] for anyone who’d prefer to look it over that way.

>> Some open problems or discussion points:
>> - You can install extensions into alternative directories using PGXS like
>> make install datadir=/else/where/share pkglibdir=/else/where/lib
>> This works. I was hoping it would work to use
>> make install prefix=/else/where
>> but that doesn't because of some details in Makefile.global. I think we can tweak that a little bit to make that work too.
>
> This works now.

I tried `prefix` with semver[3] and it did not work:

``` console
❯ make PG_CONFIG=~/dev/misc/postgres/pgsql-devel/bin/pg_config prefix=/Users/david/pgsql-test
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Werror=unguarded-availability-new -Wendif-labels -Wmissing-format-attribute -Wcast-function-type -Wformat-security -Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -Wno-cast-function-type-strict -I/opt/homebrew/opt/readline/include -I/opt/homebrew/opt/openssl/include -I/opt/homebrew/opt/libxml2/include -I/opt/homebrew/opt/icu4c/include -fvisibility=hidden -I. -I./ -I/Users/david/pgsql-test/include/server -I/Users/david/pgsql-test/include/internal -I/opt/homebrew/Cellar/icu4c(at)76/76.1_1/include -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.1.sdk -I/opt/homebrew/opt/readline/include -I/opt/homebrew/opt/openssl/include -I/opt/homebrew/opt/libxml2/include -I/opt/homebrew/opt/icu4c/include -c -o src/semver.o src/semver.c
src/semver.c:13:10: fatal error: 'postgres.h' file not found
13 | #include "postgres.h"
| ^~~~~~~~~~~~
1 error generated.
make: *** [src/semver.o] Error 1
```

It works fine without `prefix` to install into the default directories as before. It also installs fine with `datadir` and `pkglibdir`:

``` console
❯ make PG_CONFIG=~/dev/misc/postgres/pgsql-devel/bin/pg_config datadir=/Users/david/pgsql-test/share pkglibdir=/Users/david/pgsql-test/lib install
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/share/extension'
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/share/semver'
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/lib'
/opt/homebrew/bin/gmkdir -p '/Users/david/dev/misc/postgres/pgsql-devel/share/doc/semver'
/opt/homebrew/bin/ginstall -c -m 644 .//semver.control '/Users/david/pgsql-test/share/extension/'
/opt/homebrew/bin/ginstall -c -m 644 .//sql/semver--0.10.0--0.11.0.sql .//sql/semver--0.11.0--0.12.0.sql .//sql/semver--0.12.0--0.13.0.sql .//sql/semver--0.13.0--0.15.0.sql .//sql/semver--0.15.0--0.16.0.sql .//sql/semver--0.16.0--0.17.0.sql .//sql/semver--0.17.0--0.20.0.sql .//sql/semver--0.2.1--0.2.4.sql .//sql/semver--0.2.4--0.3.0.sql .//sql/semver--0.20.0--0.21.0.sql .//sql/semver--0.21.0--0.22.0.sql .//sql/semver--0.22.0--0.30.0.sql .//sql/semver--0.3.0--0.4.0.sql .//sql/semver--0.30.0--0.31.0.sql .//sql/semver--0.31.0--0.31.1.sql .//sql/semver--0.31.1--0.31.2.sql .//sql/semver--0.31.2--0.32.0.sql .//sql/semver--0.5.0--0.10.0.sql .//sql/semver--unpackaged--0.2.1.sql '/Users/david/pgsql-test/share/semver/'
/opt/homebrew/bin/ginstall -c -m 755 src/semver.dylib '/Users/david/pgsql-test/lib/'
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/lib/bitcode/src/semver'
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/lib/bitcode'/src/semver/src/
/opt/homebrew/bin/ginstall -c -m 644 src/semver.bc '/Users/david/pgsql-test/lib/bitcode'/src/semver/src/
cd '/Users/david/pgsql-test/lib/bitcode' && /opt/homebrew/Cellar/llvm/19.1.6/bin/llvm-lto -thinlto -thinlto-action=thinlink -o src/semver.index.bc src/semver/src/semver.bc
/opt/homebrew/bin/ginstall -c -m 644 .//doc/semver.mmd '/Users/david/dev/misc/postgres/pgsql-devel/share/doc/semver/'
```

But then it won’t load:

```psql
postgres=# create extension semver;
ERROR: extension "semver" has no installation script nor update path for version “0.40.0"
```

Since `semver` uses the directory parameter[4], I decided to try another C extension that doesn’t use it, envvar[5], which worked:

``` psql
postgres=# create extension envvar;
CREATE EXTENSION
```

So I suspect the issue is that, when looking for SQL files, the patch needs to use the directory parameter[4] when it’s set --- and it can be an absolute path! Honestly I think there’s a case to be made for eliminating that parameter.

The `prefix` param works with a pure SQL extension like pair[6]:

```console
❯ make PG_CONFIG=~/dev/misc/postgres/pgsql-devel/bin/pg_config prefix=/Users/david/pgsql-test
cp sql/pair.sql sql/pair--0.1.2.sql

❯ make PG_CONFIG=~/dev/misc/postgres/pgsql-devel/bin/pg_config prefix=/Users/david/pgsql-test install
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/share/extension'
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/share/extension'
/opt/homebrew/bin/gmkdir -p '/Users/david/pgsql-test/share/doc//extension'
/opt/homebrew/bin/ginstall -c -m 644 .//pair.control '/Users/david/pgsql-test/share/extension/'
/opt/homebrew/bin/ginstall -c -m 644 .//sql/pair--0.1.2.sql .//sql/pair--unpackaged--0.1.2.sql '/Users/david/pgsql-test/share/extension/'
/opt/homebrew/bin/ginstall -c -m 644 .//doc/pair.md '/Users/david/pgsql-test/share/doc//extension/‘
```

I thought it would put the files into /Users/david/pgsql-test/extension, not /Users/david/pgsql-test/share/extension? I guess that makes sense; but then perhaps the search path should be for prefixes, in which case I’d use a config like:

``` ini
extension_control_path = '/Users/david/pgsql-test:$system'
dynamic_library_path = '/Users/david/pgsql-test/lib:$libdir'
```

And the search path would append `share/extension` to each path. But then it varies from the behavior of `dynamic_library_path`. :-(

Not at all sure where the doc files should go unless, again, prefix is truly used as a prefix for all the things, which frankly seems reasonable.

I’m wondering whether there should be formal documentation of prefix, datadir, pkglibdir, etc. I haven’t noticed them in the PGXS docs[7].

>> This disables the use of dynamic_library_path, so this whole idea of installing an extension elsewhere won't work that way. The obvious solution is that extensions change this to just 'foo'. But this will require a lot updating work for many extensions, or a lot of patching by packagers.
>
>
> I have solved this by just stripping off "$libdir/" from the front of the filename. This works for now. We can think about other ways to tweak this, perhaps, but I don't see any drawback to this in practice.

I think this makes perfect sense. I presume it’s stripped out *before* replacing the MODULE_PATHNAME string in SQL files, yes?

# Patch Review

Compiles fine. All tests pass. Some comments on the docs:

> <primary><varname>extension_control_path</varname> configuration parameter</primary>

Although the current path explicitly searches for control files, I’d like to suggest a more generic name, since the point is to find extensions; control files are just the (current) method for identifying them. I suggest `extension_path`. Although given the above, maybe it should be all about prefixes.

> The value for <varname>extension_control_path</varname> must be a
> list of absolute directory paths separated by colons (or semi-colons
> on Windows). If a list element starts
> with the special string <literal>$system</literal>, the

I like this idea, though I quibble with the term `$system`, which seems like it would identify SYSCONFDIR, not SHAREDIR/extension. How about `$extdir` or, if using prefixes, `$coreprefix` or something.

> Note that the path elements should typically end in
> <literal>extension</literal> if the normal installation layouts are
> followed. (The value for <literal>$system</literal> already includes
> the <literal>extension</literal> suffix.)

In fact, if we’re using `prefix` as currently implemented, it should end in `share/extension`, no?

> Note that if you set this parameter to be able to load extensions from
> nonstandard locations, you will most likely also need to set <xref
> linkend="guc-dynamic-library-path"/> to a correspondent location, for

s/correspondent/corresponding/

> example,
> <programlisting>
> extension_control_path = '/usr/local/share/postgresql/extension:$system'
> dynamic_library_path = '/usr/local/lib/postgresql:$libdir'
> </programlisting>

This makes sense in the context of this patch, but I sure would like to see these features decoupled as much as possible.

<digression type="brief">

Hence my proposal that extensions each have their own directory. For example, via Slack Gabriele sent Peter and me a POC for loading individual extensions as mounted volumes in an OCI container. The Kubernetes config looks like this:

``` yaml
postgresql:
shared_preload_libraries:
- pg_squeeze
parameters:
extension_control_path: '/extensions/pgvector/18/share:/extensions/pgsqueeze/18/share:$system'
dynamic_library_path: '/extensions/pgvector/18/lib:/extensions/pgsqueeze/18/lib:$libdir'

extensions:
- name: pgvector
image:
reference: ghcr.io/cloudnative-pg/pgvector-testing:0.8.0
- name: pgsqueeze
image:
reference: ghcr.io/cloudnative-pg/pgsqueeze-testing:1.7
```

Which works! But it also means that new directories need to be appended to `extension_control_path` and, often `dynamic_library_path` for every extension added. It would be much nicer to just have 2-3 paths that contain extensions identified by a directory name. Then, to add a new extension, one just installs it to the appropriate prefix. (This is also yet another reason to eliminate the directory parameter[4].)

This is only additional wish I had for this feature, but I also believe we can iterate in that direction based on your patch.

</digression>

> the installation's <literal>SHAREDIR</literal> directory. By default,
> the script files are looked for in the same directory where the
> control file was found.

This could use a pointer to the impact of the directory parameter[4]. Bit of a wildcard for DBAs who want to use this feature, TBH.

Best,

David

[1]: https://commitfest.postgresql.org/51/4913/
[2]: https://github.com/theory/postgres/pull/9/files
[3]: https://github.com/theory/pg-semver
[4]: https://www.postgresql.org/docs/current/extend-extensions.html#EXTEND-EXTENSIONS-FILES-DIRECTORY
[5]: https://github.com/theory/pg-envvar
[6]: https://github.com/theory/kv-pair
[7]: https://www.postgresql.org/docs/current/extend-pgxs.html