Subject: Re: Ability to reference other extensions by schema in extension scripts

From: Sandro Santilli <[email-masked]>
Date: Tue, 28 Feb 2023 23:46:08 +0100

On Sun, Feb 26, 2023 at 01:39:24AM -0500, Regina Obe wrote:

> > 1) Just don't allow any extensions referenced by other
> >    extensions to be relocatable.
> 
> Attached is my revision 3 patch, which follows the proposed #1.
> Don't allow schema relocation of an extension if another extension
> requires it.

I've built a version of PostgreSQL with this patch applied and I
confirm it works as expected.

The "ext1" is relocatable and creates a function ext1log():

  =# create extension ext1 schema n1;
  CREATE EXTENSION

The "ext2" is relocatable and creates a function ext2log() relying
on the ext1log() function from "ext1" extension, referencing
it via @extschema:ext1@:

  =# create extension ext2 schema n2;
  CREATE EXTENSION
  =# select n2.ext2log('hello'); -- things work here
  ext1: ext2: hello

By creating "ext2", "ext1" becomes effectively non-relocatable:

  =# alter extension ext1 set schema n2;
  ERROR:  cannot SET SCHEMA of extension ext1 because other extensions
  require it
  DETAIL:  extension ext2 requires extension ext1

Drop "ext2" makes "ext1" relocatable again:

  =# drop extension ext2;
  DROP EXTENSION
  =# alter extension ext1 set schema n2;
  ALTER EXTENSION

Upon re-creating "ext2" the referenced ext1 schema will be
the correct one:

  =# create extension ext2 schema n1;
  CREATE EXTENSION
  =# select n1.ext2log('hello');
  ext1: ext2: hello
  
The code itself builds w/out warnings with:

  mkdir build
  cd build
  ../configure
  make 2> ERR # ERR is empty

The testsuite reports all successes:

  make check
  [...]
  =======================
   All 213 tests passed.
  =======================

Since I didn't see the tests for extension in there, I've also
explicitly run that portion:

  make -C src/test/modules/test_extensions/ check
  [...]
  test test_extensions              ... ok           32 ms
  test test_extdepend               ... ok           12 ms
  [...]
  =====================
   All 2 tests passed.
  =====================

As mentioned already the downside of this patch is that it would
not be possibile to change the schema of an otherwise relocatable
extension once other extension depend on it, but I can't think of
any good reason to allow that, as it would mean dependent code
would need to always dynamically determine the install location
of the objects in that extension, which sounds dangerous, security
wise.

--strk; 

  Libre GIS consultant/developer
  https://strk.kbt.io/services.html

---

From: "Regina Obe" <[email-masked]>
Date: Tue, 28 Feb 2023 17:59:16 -0500

> I've built a version of PostgreSQL with this patch applied and I confirm
it
> works as expected.
> The "ext2" is relocatable and creates a function ext2log() relying on the
> ext1log() function from "ext1" extension, referencing it via
> @extschema:ext1@:
> 
> Upon re-creating "ext2" the referenced ext1 schema will be the correct
one:
> Since I didn't see the tests for extension in there, I've also explicitly
run that
> portion:
> As mentioned already the downside of this patch is that it would not be
> possibile to change the schema of an otherwise relocatable extension once
> other extension depend on it, but I can't think of any good reason to
allow
> that, as it would mean dependent code would need to always dynamically
> determine the install location of the objects in that extension, which
sounds
> dangerous, security wise.
> 
> --strk;
> 

Oops I had forgotten to submit the updated patch strk was testing against in
my fork.
He had asked me to clean up the warnings off list and the description.

Attached is the revised.
Thanks strk for the patient help and guidance.

Thanks,
Regina

---

From: Sandro Santilli <[email-masked]>
Date: Tue, 28 Feb 2023 23:43:13 +0000

The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

I've applied the patch attached to message https://www.postgresql.org/message-id/000401d94bc8%2448dff700%24da9fe500%24%40pcorp.us (md5sum a7d45a32c054919d94cd4a26d7d07c20) onto current tip of the master branch being 128dd9f9eca0b633b51ffcd5b0f798fbc48ec4c0

The review written in https://www.postgresql.org/message-id/20230228224608.ak7br5shev4wic5a%40c19 all still applies.

The `make installcheck-world`  test fails for me but the failures seem unrelated to the patch (many occurrences of "+ERROR:  function pg_input_error_info(unknown, unknown) does not exist" in the regression.diff).

Documentation exists for the new feature

The new status of this patch is: Ready for Committer