RE: Ability to reference other extensions by schema in extension scripts

> 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 ?
>

I like that idea. It's only been ever used for tables I think, but I don't
see why it wouldn't apply in this case as the concept is kinda the same.
Only concern if other parts rely on this being 0.

The other question, should this just update the existing DEPENDENCY_NORMAL
extension or add a new DEPENDENCY_NORMAL between the extensions with
subid=1?

> 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:ext1(at)(dot)ext1log()
>

This would be an okay solution to me too if everyone is okay with it.

> 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
>

Oops. I don't know why I thought the normal dependency would protect
against this. I should have tested that. So DEPENDENCY_AUTO is not an
option to use and creating a new type of dependency seems like over stepping
the bounds of this patch.

> 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

So in conclusion we have 3 possible paths to go with this

1) Just don't allow any extensions referenced by other extensions to be
relocatable.
It will show a message something like
"SET SCHEMA not allowed because other extensions depend on it"
Given that if you don't specify relocatable in you .control file, the assume
is relocatable = false , this isn't too far off from standard protocol.

2) Use objsubid=1 to denote that another extension explicitly references the
schema of another extension so setting schema of other extension is not
okay. So instead of introducing another dependency, we'd update the
DEPENDENCY_NORMAL one between the two schemas with objsubid=1 instead of 0.

This has 2 approaches:

a) Update the existing DEPENDENCY_NORMAL between the two extensions setting
the objsubid=1

or
b) Create a new DEPEDENCY_NORMAL between the two extensions with objsubid=1

I'm not sure if either has implications in backup / restore . I suspect b
would be safer since I suspect objsubid might be checked and this
dependency only needs checking during SET SCHEMA time.

3) Create a whole new DEPENDENCY type, perhaps calling it something like
DEPENDENCY_EXTENSION_SCHEMA

4) Just don't allow @extschema:<reqextension>@ syntax to be used unless the
<reqextension> is marked as relocatable=false. This one I don't like
because it doesn't solve my fundamental issue of

postgis_tiger_geocoder relying on fuzzystrmatch, which is marked as
relocatable.

The main issue I was trying to solve is my extension references
fuzzystrmatch functions in a function used for functional indexes, and this
fails restore of table indexes because I can't schema qualify the
fuzzystrmatch extension in the backing function.

If no one has any opinion, I'll go with option 1 which is the one that strk
had actually proposed before and seems least programmatically invasive, but
perhaps more annoying user facing.

My preferred would be #2

Thanks,
Regina