"Regina Obe" <lr@pcorp.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.
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.
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
