public inbox for [email protected]  

help / color / mirror / Atom feed

From: Sandro Santilli <[email protected]>
To: Regina Obe <[email protected]>
Cc: 'Tom Lane' <[email protected]>
Cc: [email protected]
Subject: Re: Ability to reference other extensions by schema in extension scripts
Date: Thu, 23 Feb 2023 19:39:06 +0100
Message-ID: <20230223183906.6rhtybwdpe37sri7@c19> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<20221215085234.rpjzlm7hmsmbanqv@c19>
	<[email protected]>

On Mon, Feb 06, 2023 at 05:19:39AM -0500, Regina Obe wrote:
> 
> Attached is a revised version of the original patch. It is revised to
> prevent 
> 
> ALTER EXTENSION .. SET SCHEMA  if there is a dependent extension that 
> references the extension in their scripts using @extschema:extensionname@
> It also adds additional tests to verify that new feature.
> 
> In going thru the code base, I was tempted to add a new dependency type
> instead of using the existing DEPENDENCY_AUTO.  I think this would be
> cleaner, but I felt that was overstepping the area a bit, since it requires
> making changes to dependency.h and dependency.c
> 
> My main concern with using DEPENDENCY_AUTO is because it was designed for
> cases where an object can be dropped without need for CASCADE.  In this
> case, we don't want a dependent extension to be dropped if it's required is
> dropped.  However since there will already exist 
> a DEPENDENCY_NORMAL between the 2 extensions, I figure we are protected
> against that issue already.

I was thinking: how about using the "refobjsubid" to encode the
"level" of dependency on an extension ? Right now "refobjsubid" is
always 0 when the referenced object is an extension.
Could we consider subid=1 to mean the dependency is not only
on the extension but ALSO on it's schema location ?

Also: should we really allow extensions to rely on other extension
w/out fully-qualifying calls to their functions ? Or should it be
discouraged and thus forbidden ? If we wanted to forbid it we then
would not need to encode any additional dependency but rather always
forbid `ALTER EXTENSION .. SET SCHEMA` whenever the extension is
a dependency of any other extension.

On the code in the patch itself, I tried with this simple use case:

  - ext1, relocatable, exposes an ext1log(text) function

  - ext2, relocatable, exposes an ext2log(text) function
    calling @extschema:[email protected]()

What is not good:

	- Drop of ext1 automatically cascades to drop of ext2 without even a notice:

		test=# create extension ext2 cascade;
		NOTICE:  installing required extension "ext1"
		CREATE EXTENSION
		test=# drop extension ext1;
		DROP EXTENSION -- no WARNING, no NOTICE, ext2 is gone

What is good:

	- ext1 cannot be relocated while ext2 is loaded:

		test=# create extension ext2 cascade;
		NOTICE:  installing required extension "ext1"
		CREATE EXTENSION
		test=# alter extension ext1 set schema n1;
		ERROR:  Extension can not be relocated because dependent extension references it's location
		test=# drop extension ext2;
		DROP EXTENSION
		test=# alter extension ext1 set schema n1;
		ALTER EXTENSION

--strk;

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

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Ability to reference other extensions by schema in extension scripts
  In-Reply-To: <20230223183906.6rhtybwdpe37sri7@c19>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox