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

From: "Regina Obe" <lr(at)pcorp(dot)us>
To: "'Tom Lane'" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "'Gregory Stark \(as CFM\)'" <stark(dot)cfm(at)gmail(dot)com>, "'Sandro Santilli'" <strk(at)kbt(dot)io>, <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "'Regina Obe'" <r(at)pcorp(dot)us>
Subject: RE: Ability to reference other extensions by schema in extension scripts
Date: 2023-03-10 21:05:46
Message-ID: 004b01d95394$16123ee0$4236bca0$@pcorp.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> "Regina Obe" <lr(at)pcorp(dot)us> writes:
> > [ 0005-Allow-use-of-extschema-reqextname-to-reference.patch ]
>
> I took a look at this. I'm on board with the feature design, but not so
much
> with this undocumented restriction you added to ALTER EXTENSION SET
> SCHEMA:
>
> + /* If an extension requires this extension
> + * do not allow relocation */
> + if (pg_depend->deptype == DEPENDENCY_NORMAL &&
> pg_depend->classid == ExtensionRelationId){
> + dep.classId = pg_depend->classid;
> + dep.objectId = pg_depend->objid;
> + dep.objectSubId = pg_depend->objsubid;
> + ereport(ERROR,
> +
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot SET SCHEMA of
> extension %s because other extensions require it",
> + NameStr(extForm-
> >extname)),
> + errdetail("%s requires extension
%s",
> +
> getObjectDescription(&dep, false),
> +NameStr(extForm->extname))));
>
> That seems quite disastrous for usability, and it's making an assumption
> unsupported by any evidence: that it will be a majority use-case for
> dependent extensions to have used @extschema:myextension@ in a way that
> would be broken by ALTER EXTENSION SET SCHEMA.
>
> I think we should just drop this. It might be worth putting in some
> documentation notes about the hazard, instead.
>
That was my thought originally too and also given the rarity of people
changing schemas
I wasn't that bothered with not forcing this. Sandro was a bit more
bothered by not forcing it and given the default for extensions is not
relocatable, we didn't see that much of an issue with it.

> If you want to work harder, perhaps a reasonable way to deal with the
issue
> would be to allow dependent extensions to declare that they don't want
your
> extension relocated. But I do not think it's okay to make that the
default
> behavior, much less the only behavior.

I had done that in one iteration of the patch.
We discussed this here
https://www.postgresql.org/message-id/000001d949ad%241159adc0%24340d0940%24%
40pcorp.us

and here
https://www.postgresql.org/message-id/20230223183906.6rhtybwdpe37sri7%40c19

- the main issue I ran into is I have to introduce another dependency type
or go with Sandro's idea of using refsubobjid for this purpose. I think
defining a new dependency type is less likely to cause unforeseen
complications elsewhere, but did require me to expand the scope (to make
changes to pg_depend). Which I am fine with doing, but didn't want to over
extend my reach too much.

One of my revisions tried to use DEPENDENCY_AUTO which did not work (as
Sandro discovered) and I had some other annoyances with lack of helper
functions
https://www.postgresql.org/message-id/000401d93a14%248647f540%2492d7dfc0%24%
40pcorp.us

key point:
"Why isn't there a variant getAutoExtensionsOfObject take a DEPENDENCY type
as an option so it would be more useful or is there functionality for that I
missed?"

> And really, since we've gotten along without it so far, I'm not sure that
it's
> necessary to have it.
>
> Another thing that's bothering me a bit is the use of
get_required_extension
> in execute_extension_script. That does way more than you really need, and
> passing a bunch of bogus parameter values to it makes me uncomfortable.
> The callers already have the required extensions' OIDs at hand; it'd be
better
> to add that list to execute_extension_script's API instead of redoing the
> lookups.
>
> regards, tom lane

So you are proposing I change the execute_extension_scripts input args to
take more args?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2023-03-10 21:42:35 Lock mode in ExecMergeMatched()
Previous Message David Zhang 2023-03-10 20:58:23 Re: [BUG] pg_stat_statements and extended query protocol