Subject: Re: RFC: Additional Directory for Extensions

From: Matheus Alcantara <[email-masked]>
Date: Mon, 10 Mar 2025 17:25:40 -0300

Hi,

Thanks for reviewing and suggestions!

On Thu, Mar 6, 2025 at 10:46 AM Peter Eisentraut <[email-masked]> wrote:
> This looks very good to me.  I have one issue to point out:  The logic
> in get_extension_control_directories() needs to be a little bit more
> careful to align with the rules in find_in_path().  For example, it
> should use first_path_var_separator() to get the platform-specific path
> separator, and probably also substitute_path_macro() and
> canonicalize_path() etc., to keep everything consistent.
>
I fixed this hardcoded path separator issue on the TAP test and forgot
to fix it also on code, sorry, fixed on this new version.

I also spent some time investigating why the tests on Windows were still
passing even using a wrong path separator.

Consider extension_control_path = '$system;C:\custom\path'

When running on Windows, the get_extension_control_directories was
returning [$system;C:, \custom\path] and for somehow the \custom\path
was successfully being read and since the tests was only referencing the
extension on this custom path everything was passing, but querying for
an extension that is only on $system was resulting in an empty query
result. In the attached patch I also included a new test case to query
on pg_available_extensions for an extension that is installed on the
$system, so we can ensure that extensions in both paths can be used
correctly.

> (Maybe it would be ok to move the function to dfmgr.c to avoid having
> to export too many things from there.)
>
I've exported substitute_path_macro because adding a new function on
dfmgr would require #include nodes/pg_list.h and I'm not sure what
approach would be better, please let me know what you think.

-- 
Matheus Alcantara

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 11 Mar 2025 16:58:55 +0100

> approach would be better, please let me know what you think.

Yes, that structure looks ok.  But you can remove one level of block in 
get_extension_control_directories().

I found a bug that was already present in my earlier patch versions:

@@ -423,7 +424,7 @@ find_extension_control_filename(ExtensionControlFile 
*control)
     ecp = Extension_control_path;
     if (strlen(ecp) == 0)
         ecp = "$system";
-   result = find_in_path(basename, Extension_control_path, 
"extension_control_path", "$system", system_dir);
+   result = find_in_path(basename, ecp, "extension_control_path", 
"$system", system_dir);

Without this, it won't work if you set extension_control_path empty. 
(Maybe add a test for that?)

I think this all works now, but I think the way 
pg_available_extensions() works is a bit strange and inefficient.  After 
it finds a candidate control file, it calls 
read_extension_control_file() with the extension name, that calls 
parse_extension_control_file(), that calls 
find_extension_control_filename(), and that calls find_in_path(), which 
searches that path again!

There should be a simpler way into this.  Maybe 
pg_available_extensions() should fill out the ExtensionControlFile 
structure itself, set ->control_dir with where it found it, then call 
directly to parse_extension_control_file(), and that should skip the 
finding if the directory is already set.  Or something similar.

---

From: Matheus Alcantara <[email-masked]>
Date: Wed, 12 Mar 2025 10:17:19 -0300

> get_extension_control_directories().
Sorry, missed during debugging. Fixed

> (Maybe add a test for that?)
Fixed, and also added a new test case for this.

> finding if the directory is already set.  Or something similar.
Good catch. I fixed this by creating a new function to construct the
ExtensionControlFile and changed the pg_available_extensions to set the
control_dir. The read_extension_control_file was also changed to just
call this new function constructor. I implemented the logic to check if
the control_dir is already set on parse_extension_control_file because
it seems to me that make more sense to not call
find_extension_control_filename instead of putting this logic there
since we already set the control_dir when we find the control file, and
having the logic to set the control_dir or skip the find_in_path seems
more confusing on this function instead of on
parse_extension_control_file. Please let me know what you think.

-- 
Matheus Alcantara

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 19 Mar 2025 07:42:44 +0100

> parse_extension_control_file. Please let me know what you think.

Committed that, thanks.

A small tweak I made was to replace palloc+snprintf by psprintf.  Maybe 
you were not aware that that function exists.

I also simplified the error handling in parse_extension_control_file() a 
bit.  If we pass in a control directory (which is the new code we're 
adding), then we can assume that we already found the file earlier, and 
then if we now don't find it, then we should just report the file system 
error instead of the "you should install this extension first" error. 
It's kind of a "can't happen" error anyway, so the different is small.

---

From: Gabriele Bartolini <[email-masked]>
Date: Wed, 19 Mar 2025 08:03:08 +0100

Thanks everyone for making this happen.

Ciao,
Gabriele

-- 
Gabriele Bartolini
VP, Chief Architect, Kubernetes
enterprisedb.com

---

From: Christoph Berg <[email-masked]>
Date: Wed, 19 Mar 2025 15:58:59 +0100

Re: Peter Eisentraut
> Committed that, thanks.

Awesome, thanks!

It works perfectly for the Debian "test extension packages at build
time" use case, replacing our old extension_destdir patch.

PKGARGS="--pgoption extension_control_path=$PWD/debian/$PACKAGE/usr/share/postgresql/$v/extension:\$system --pgoption dynamic_library_path=$PWD/debian/$PACKAGE/usr/lib/postgresql/$v/lib:/usr/lib/postgresql/$v/lib"

https://salsa.debian.org/postgresql/postgresql-common/-/commit/3792eea42e4dcef39b5c8d99f63deb8091ef9c46

Christoph

---

From: "David E. Wheeler" <[email-masked]>
Date: Wed, 19 Mar 2025 13:29:29 -0400

> Committed that, thanks.

🎉

I’ve been meaning to test the patch again, so here goes.

First thing I notice is that prefix= uses the magic to insert “postgresql” into the path if it’s not already there:

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

I think this should at least be documented, but generally feels unexpected to me. I’ve attached a patch that fleshes out the docs, along with an example of setting `extension_control_path` and `dynamic_library_path` to use the locations. It might not have the information right about the need for “postgresql” or “pgsql” in the path. Back in 2003[1] it was just “postgres”, but I couldn’t find the logic for it just now.

Everything else works very nicely except for extensions that use the Makefile `MODULEDIR` variable to install all of the share files except the control file into a particular directory, and the `directory` in the control file so that the files can be found. Here’s semver[2], which has both:

```console
❯ make PG_CONFIG=~/dev/c/postgres/pgsql-devel/bin/pg_config prefix=/Users/david/Downloads/postgresql install
/opt/homebrew/bin/gmkdir -p '/Users/david/Downloads/postgresql/share/extension'
/opt/homebrew/bin/gmkdir -p '/Users/david/Downloads/postgresql/share/semver'
/opt/homebrew/bin/gmkdir -p '/Users/david/Downloads/postgresql/lib'
/opt/homebrew/bin/gmkdir -p '/Users/david/Downloads/postgresql/share/doc//semver'
/opt/homebrew/bin/ginstall -c -m 644 .//semver.control '/Users/david/Downloads/postgresql/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.32.0--0.32.1.sql .//sql/semver--0.32.1--0.40.0.sql .//sql/semver--0.32.1.sql .//sql/semver--0.40.0.sql .//sql/semver--0.5.0--0.10.0.sql .//sql/semver--unpackaged--0.2.1.sql .//sql/semver.sql  '/Users/david/Downloads/postgresql/share/semver/'
/opt/homebrew/bin/ginstall -c -m 755  src/semver.dylib '/Users/david/Downloads/postgresql/lib/'
/opt/homebrew/bin/gmkdir -p '/Users/david/Downloads/postgresql/lib/bitcode/src/semver'
/opt/homebrew/bin/gmkdir -p '/Users/david/Downloads/postgresql/lib/bitcode'/src/semver/src/
/opt/homebrew/bin/ginstall -c -m 644 src/semver.bc '/Users/david/Downloads/postgresql/lib/bitcode'/src/semver/src/
cd '/Users/david/Downloads/postgresql/lib/bitcode' && /opt/homebrew/Cellar/llvm/19.1.7_1/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/Downloads/postgresql/share/doc//semver/‘
```

Following `MODULEDIR=semver`, it puts the SQL files into `share/semver/` instead of `share/extension/`, as expected, but then, even though the control file has `directory=semver`, it can’t load them:

```pgsql
david=# create extension semver;
ERROR:  could not open directory "/Users/david/dev/c/postgres/pgsql-devel/share/semver": No such file or directory
```

Looks like it’s only looking in the `semver` subdirectory under $libdir and not the whole path.

But given that the `directory` variable in the control file can be a full path, I don’t see that there’s much of a way to generalize a solution. I guess there are three options:

1. If directory is a full path, try to load the files there. It probably already works that way, though I haven’t tired it.

2. If the directory is not a full path, check for it under each directory in `extension_control_path`? But no, that points to `share/extension`, not `share`, so it can’t really searched unless it also lops off `extension` from the end of each path.

3. Drop support for MODULEDIR and directory.

I think I’d opt for #3, personally, just to simplify things.

Anyway, I then built envvar, a C extension with no `directory` configuration, and it worked perfectly.

I will say, though, that I will kind of miss being able to run `make install` without first running `make`, as the `prefix` variable does not work with `make`.

Best,

David

[1]: https://postgr.es/m/[email-masked]
[2]: https://github.com/theory/pg-semver/
[3]: https://github.com/theory/pg-envvar

---

From: Tom Lane <[email-masked]>
Date: Wed, 19 Mar 2025 14:55:58 -0400

> Committed that, thanks.

Buildfarm member snakefly doesn't like this too much.  Since no other
animals have failed, I guess it must be about local conditions on
that machine, but the report is pretty opaque:

# +++ tap check in src/test/modules/test_extensions +++

#   Failed test '$system extension is installed correctly on pg_available_extensions'
#   at t/001_extension_control_path.pl line 69.
#          got: 'f'
#     expected: 't'

#   Failed test '$system extension is installed correctly on pg_available_extensions with empty extension_control_path'
#   at t/001_extension_control_path.pl line 76.
#          got: 'f'
#     expected: 't'
# Looks like you failed 2 tests of 5.
[06:43:53] t/001_extension_control_path.pl .. 
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/5 subtests

Looking at the test, it presupposes that "amcheck" must be an
available extension.  I do not see anything that guarantees
that that's so, though.  It'd fail if contrib hasn't been
installed.  Is there a reason to use "amcheck" rather than
something more certainly available, like "plpgsql"?

			regards, tom lane

---

From: Matheus Alcantara <[email-masked]>
Date: Wed, 19 Mar 2025 16:25:54 -0300

> something more certainly available, like "plpgsql"?

There is no specific reason to use "amcheck" instead of "plpgsql". Attached a
patch with this change, sorry about that.

(Not sure if we should also improve the message to make the test failure less
opaque?)

-- 
Matheus Alcantara

---

From: Tom Lane <[email-masked]>
Date: Wed, 19 Mar 2025 15:42:46 -0400

> (Not sure if we should also improve the message to make the test failure less

Yeah, I was wondering how to do that.  The earlier tests in that
script show the whole row from pg_available_extensions, not just a
bool ... but that doesn't help if the problem is we don't find a row.

			regards, tom lane

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 20 Mar 2025 11:01:37 +0100

> patch with this change, sorry about that.

Committed.

I was able to reproduce the problem from scratch using:

./configure ...
make  # no contrib
make -C src/test/modules/test_extensions check

So it depended on in which order you build the various components.

---

From: Tom Lane <[email-masked]>
Date: Thu, 20 Mar 2025 10:35:51 -0400

Peter Eisentraut <[email-masked]> writes:
>>> Buildfarm member snakefly doesn't like this too much.

> So it depended on in which order you build the various components.

That makes sense, but I wonder how snakefly hit it while other BF
animals did not.  It's running a reasonably up-to-date BF client
version and there's nothing odd-looking about its configuration.

Anyway, I see snakefly is green now so that tweak did fix it.

			regards, tom lane

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 20 Mar 2025 10:53:17 -0400

On 2025-03-19 We 2:55 PM, Tom Lane wrote:
> Peter Eisentraut<[email-masked]> writes:
>> Committed that, thanks.

I think something else must be going on. The failure in question came 
after the step "install-contrib" succeeded, and the log file for that shows:

make[1]: Entering directory `/opt/postgres/build-farm-18/HEAD/pgsql.build/contrib/amcheck'
/usr/bin/mkdir -p '/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/lib'
/usr/bin/mkdir -p '/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/share/extension'
/usr/bin/mkdir -p '/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/share/extension'
/usr/bin/install -c -m 755  amcheck.so '/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/lib/amcheck.so'
/usr/bin/install -c -m 644 ./amcheck.control '/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/share/extension/'
/usr/bin/install -c -m 644 ./amcheck--1.3--1.4.sql ./amcheck--1.2--1.3.sql ./amcheck--1.1--1.2.sql ./amcheck--1.0--1.1.sql ./amcheck--1.0.sql  '/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/share/extension/'
make[1]: Leaving directory `/opt/postgres/build-farm-18/HEAD/pgsql.build/contrib/amcheck'

(wondering if this another of these cases where the "path includes postgres" thing bites us, and we're looking in the wrong place)

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

---

From: Andrew Dunstan <[email-masked]>
Date: Thu, 20 Mar 2025 18:38:10 -0400

> (wondering if this another of these cases where the "path includes postgres" thing bites us, and we're looking in the wrong place)

Nope, testing shows it's not that, so I am rather confused about what 
was going on.

cheers

andrew

---

From: Matheus Alcantara <[email-masked]>
Date: Fri, 21 Mar 2025 12:52:55 -0300

On Thu, Mar 20, 2025 at 7:38 PM Andrew Dunstan <[email-masked]> wrote:
>> I think something else must be going on. The failure in question came after the step "install-contrib" succeeded, and the log file for that shows:
>>
>>
> Nope, testing shows it's not that, so I am rather confused about what was going on.
>

I'm not sure if I'm checking on the right place [1] but it seems that the
Contrib and ContribInstall is executed after Check step which causes this test
failure?

'steps_completed' => [
                      'SCM-checkout',
                      'Configure',
                      'Build',
                      'Check',
                      'Contrib',
                      'TestModules',
                      'Install',
                      'ContribInstall',
                      'TestModulesInstall',
                      'MiscCheck',
                      ...
                      ]

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snakefly&dt=2025-03-20%2009%3A46%3A05
-- 
Matheus Alcantara

---

From: Andrew Dunstan <[email-masked]>
Date: Fri, 21 Mar 2025 12:38:58 -0400

> [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snakefly&dt=2025-03-20%2009%3A46%3A05

No. In the buildfarm, the Check step only runs the core regression 
tests, not any TAP tests. The above shows fairly clearly that the 
failure occurred after the ContribInstall step, which is what's puzzling me.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Tom Lane <[email-masked]>
Date: Fri, 21 Mar 2025 12:42:28 -0400

> Contrib and ContribInstall is executed after Check step which causes this test

No, this is not failing in Check.

I did just notice a clue though: on snakefly, the failing step's
log [1] includes

make[1]: Leaving directory `/opt/postgres/build-farm-18/HEAD/pgsql.build/src/backend'
rm -rf '/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install
/usr/bin/mkdir -p '/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/log
make -C '../../../..' DESTDIR='/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install install >'/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/log/install.log 2>&1
make -j1  checkprep >>'/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/log/install.log 2>&1
PATH="/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/bin:/opt/postgres/build-farm-18/HEAD/pgsql.build/src/test/modules/test_extensions:$PATH" LD_LIBRARY_PATH="/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/lib:$LD_LIBRARY_PATH" INITDB_TEMPLATE='/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/initdb-template  initdb --auth trust --no-sync --no-instructions --lc-messages=C --no-clean '/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/initdb-template >>'/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/log/initdb-template.log 2>&1
echo "# +++ regress check in src/test/modules/test_extensions +++" && PATH="/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/bin:/opt/postgres/build-farm-18/HEAD/pgsql.build/src/test/modules/test_extensions:$PATH" LD_LIBRARY_PATH="/opt/postgres/build-farm-18/HEAD/pgsql.build/tmp_install/opt/postgres/build-farm-18/HEAD/inst/lib:$LD_LIBRARY_PATH" INITDB_TEMPLATE='/opt/postgres/build-farm-18/HEAD/pgsql.build'/tmp_install/initdb-template  ../../../../src/test/regress/pg_regress --temp-instance=./tmp_check --inputdir=. --bindir=  --temp-config=/opt/postgres/build-farm-18/tmp/buildfarm-C9Iy3s/bfextra.conf  --no-locale --port=5678 --dbname=contrib_regression test_extensions test_extdepend
# +++ regress check in src/test/modules/test_extensions +++
# initializing database system by running initdb

showing that the step made its own tmp_install, and that only the core
"install" process was executed, so the lack of amcheck in that install
tree is not surprising.  But concurrent runs on other animals, eg [2],
don't show a tmp_install rebuild happening.  So those are using an
installation tree that *does* include contrib modules.

So what this comes down to is "why is snakefly doing a fresh install
here?".  I don't know the buildfarm client well enough to identify
probable causes.  I do note that Makefile.global.in conditionalizes
tmp_install rebuild on several variables:

temp-install: | submake-generated-headers
ifndef NO_TEMP_INSTALL
ifneq ($(abs_top_builddir),)
ifeq ($(MAKELEVEL),0)
	rm -rf '$(abs_top_builddir)'/tmp_install
	$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
	$(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
	$(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1

I think we've had trouble before with that MAKELEVEL test...

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=snakefly&dt=2025-03-20%2009%3A46%3A05&stg=module-test_extensions-check
[2] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=alligator&dt=2025-03-19%2006%3A10%3A38&stg=module-test_extensions-check

---

From: Matheus Alcantara <[email-masked]>
Date: Fri, 21 Mar 2025 18:52:20 -0300

Hi David, thanks for testing!

On Wed, Mar 19, 2025 at 2:29 PM David E. Wheeler <[email-masked]> wrote:
>
> First thing I notice is that prefix= uses the magic to insert
> “postgresql” into the path if it’s not already there:
>
> ``` console
> I think this should at least be documented, but generally feels
> unexpected to me. I’ve attached a patch that fleshes out the docs,
> along with an example of setting `extension_control_path` and
> `dynamic_library_path` to use the locations. It might not have the
> information right about the need for “postgresql” or “pgsql” in the
> path.

Did you miss to attach the patch?

> Everything else works very nicely except for extensions that use the
> Makefile `MODULEDIR` variable to install all of the share files except
> the control file into a particular directory, and the `directory` in
> the control file so that the files can be found. Here’s semver[2],
> which has both:
>
> ```console
> Following `MODULEDIR=semver`, it puts the SQL files into
> `share/semver/` instead of `share/extension/`, as expected, but then,
> even though the control file has `directory=semver`, it can’t load
> them:
>
> ```pgsql

I've managed to reproduce the issue. The problem is on get_ext_ver_list
which calls get_extension_script_directory that try to search for .sql
files only on $sharedir.

> But given that the `directory` variable in the control file can be a
> full path, I don’t see that there’s much of a way to generalize a
> solution. I guess there are three options:
>
> 1. If directory is a full path, try to load the files there. It
> probably already works that way, though I haven’t tired it.
>
Yes, if the directory is a full path it try to load the files from
there. It is implemented on get_extension_script_directory.

> 2. If the directory is not a full path, check for it under each
> directory in `extension_control_path`? But no, that points to
> `share/extension`, not `share`, so it can’t really searched unless it
> also lops off `extension` from the end of each path.

Maybe we could make the "extension" part of the extension control path
explicitly, like Peter has mentioned in his first patch version [1]?.
If "directory" is not set we could use "extension" otherwise use the
"directory" as a path suffix when searching on extension_control_path?

[1] https://www.postgresql.org/message-id/0d384836-7e6e-4932-af3b-8dad1f6fee43%40eisentraut.org

-- 
Matheus Alcantara

---

From: "David E. Wheeler" <[email-masked]>
Date: Fri, 21 Mar 2025 18:05:34 -0400

> Did you miss to attach the patch?

No. You can see it in the archive[1]. Direct link[2].

> "directory" as a path suffix when searching on extension_control_path?

So, omit “extension” from the path options, append it to search for control files, and then append the directory value (if not absolute) if it exists to look for files, and otherwise append “extensions” to find them, too. I think that makes sense.

Essentially it becomes a SHAREDIR search path.

Best,

David

[1]: https://postgr.es/m/[email-masked]
[2]: https://www.postgresql.org/message-id/attachment/174397/v1-0001-Flesh-out-docs-for-the-prefix-make-variable.patch

---

From: Andrew Dunstan <[email-masked]>
Date: Sat, 22 Mar 2025 09:28:41 -0400

On 2025-03-21 Fr 12:42 PM, Tom Lane wrote:
> Matheus Alcantara<[email-masked]> writes:
>> On Thu, Mar 20, 2025 at 7:38 PM Andrew Dunstan<[email-masked]> wrote:

Good catch. This is happening because the owner hasn't updated the 
animal to REL_19_1. In 19 and 19.1 we fixed detection of exiting 
installs to take account of the 'Is there postgres or pgsql in the 
prefix' issue. So it was looking in the wrong place.

cheers

andrew

---

From: "David E. Wheeler" <[email-masked]>
Date: Thu, 24 Apr 2025 18:59:12 -0400

> I think this should at least be documented, but generally feels unexpected to me. I’ve attached a patch that fleshes out the docs, along with an example of setting `extension_control_path` and `dyn…

Here’s a rebase.

Best,

David

---

From: Christoph Berg <[email-masked]>
Date: Fri, 25 Apr 2025 13:33:22 +0200

Re: David E. Wheeler
> +<programlisting>
> +make install prefix=/etc/postgresql

I'd use /usr/local/postgresql there. "/etc" is just wrong.

> +</programlisting>
> +    This will install the control SQL files into
> +    <literal>/etc/postgresql/share</literal> and shared modules into
> +    <literal>/etc/postgresql/lib</literal>. If the prefix does not
> +    include the strings <literal>postgresql</literal> or

Just "postgres", see src/Makefile.global.in:86.

> +    <literal>pgsql</literal>, such as:
> +<programlisting>
> +make install prefix=/etc/extras

/usr/local/extras

> +</programlisting>
> +    Then the <literal>postgresql</literal> directory will be appended io the
> +    prefix, installing the control SQL files into

"the extension control and SQL files"

> +    <literal>/etc/extras/postgresql/share</literal> and shared modules into

.../postgresql/share/extension because ...

> +    <literal>/etc/extras/postgresql/lib</literal>. Either way, you'll need to
> +    set <xref linkend="guc-extension-control-path"/> and <xref
> +    linkend="guc-dynamic-library-path"/> to allow
> +    <productname>PostgreSQL</productname> to find the files:
> +</programlisting>
> +extension_control_path = '/etc/extras/postgresql/share/extension:$system'

... it's used here.

Christoph

---

From: "David E. Wheeler" <[email-masked]>
Date: Fri, 25 Apr 2025 15:23:47 -0400

> I'd use /usr/local/postgresql there. "/etc" is just wrong.

Thank you for the review. Here’s v3*.

Best,

David

* Also reviewable as a GitHub PR[1].

[1]: https://github.com/theory/postgres/pull/10

---

From: "David E. Wheeler" <[email-masked]>
Date: Mon, 28 Apr 2025 17:14:58 -0400

> Thank you for the review. Here’s v3*.

V4 removes “/extension” from the end of the `extension_control_path` value.

Best,

David

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 1 May 2025 13:50:37 +0200

> V4 removes “/extension” from the end of the `extension_control_path` value.

The documentation in config.sgml says:

     Note that the path elements should typically end in
     <literal>extension</literal> if the normal installation layouts are
     followed.

So I think your change here between v3 and v4 is incorrect.

---

From: "David E. Wheeler" <[email-masked]>
Date: Thu, 1 May 2025 10:31:28 -0400

> So I think your change here between v3 and v4 is incorrect.

Right, sorry, forgot about that, I updated it in anticipation of Matheus’s patch[1] being committed.

So v3 is fine for now, but if that patch is committed, we’ll need to reconcile those docs.

Best,

David

[1]: https://postgr.es/m/CAFY6G8dUXHRii5rNy7V8WmBrmBwp9W7y3g+HL6Tn-Lu8KkvK=[email-masked]

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 1 May 2025 22:24:20 +0200

> So v3 is fine for now, but if that patch is committed, we’ll need to reconcile those docs.

I see.  I have committed it now describing the current state.

Btw., the shown directory names that illustrate how "postgresql" is 
appended were not correct.  I have corrected that.

---

From: "David E. Wheeler" <[email-masked]>
Date: Thu, 1 May 2025 17:01:52 -0400

On May 1, 2025, at 16:24, Peter Eisentraut <[email-masked]> wrote:

> Btw., the shown directory names that illustrate how "postgresql" is appended were not correct. I have corrected that.

Thank you. I gotta say I find them confusing TBH.

Best,

David

---

From: "David E. Wheeler" <[email-masked]>
Date: Fri, 2 May 2025 12:48:35 -0400

> I see.  I have committed it now describing the current state.

Quick follow-up to tweak a couple of commas.

--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1813,8 +1813,8 @@ include $(PGXS)
 
    <para>
     You can select a separate directory prefix in which to install your
-    extension's files, by setting the <command>make</command> variable
-    <varname>prefix</varname> when executing <literal>make install</literal>
+    extension's files by setting the <command>make</command> variable
+    <varname>prefix</varname> when executing <literal>make install</literal>,
     like so:
 <programlisting>
 make install prefix=/usr/local/postgresql

Best,

David