On Fri, 27 Sept 2024 at 14:00, Tomas Vondra <to...@vondra.me> wrote: > One thing that's not quite clear to me is what's the correct way for > existing extensions to switch to an "owned schema". Let's say you have > an extension. How do you transition to this? Can you just add it to the > control file and then some magic happens?
Sadly no. As currently implemented this feature can only really be used by new extensions. There is no upgrade path to it for existing extensions. This is fairly common btw, the only field in the control file that ever gets used after the taken from the control file for ALTER EXTENSION is the version field. e.g. if you change the schema field in the control file of an already installed extension, the extension will not be moved to the new schema unless you DROP + CREATE EXTENSION. After this message I tried messing around a bit with changing an existing extension to become an owned schema (either with a new command or as part of ALTER EXTENSION UPDATE). But it's not as trivial as I hoped for a few reasons: 1. There are multiple states that extensions are currently in all of which need somewhat different conversion strategies. Specifically: relocatable/non-relocatable & schema=pg_catalog/public/actual_extension_schema. 2. There are two important assumptions on the schema for an owned_schema, which we would need to check on an existing schema converting it: a. The owner of the extension should be the owner of the schema b. There are no other objects in the schema appart from the extension. I'll probably continue some efforts to allow for migration, because I agree it's useful. But I don't think it's critical for this feature to be committable. Even if this can only be used by new extensions (that target PG18+ only), I think that would already be very useful. i.e. if in 5 years time I don't have to worry about these security pitfalls for any new extensions that I write then I'll be very happy. Also if an extension really wants to upgrade to an owned schema, then that should be possible by doing some manual catalog hackery in its extension update script, because that's effectively what any automatic conversion would also do. > A couple minor comments: > Doesn't "extension is owned_schema" sound a bit weird? Updated the docs to address all of your feedback I think. > Also, perhaps "dedicated_schema" would be better than "owned_schema"? I > mean, the point is not that it's "owned" by the extension, but that > there's nothing else in it. But that's nitpicking. I agree that's probably a better name. Given the amount of effort necessary to update everything I haven't done that yet. I'll think a bit if there's a name I like even better in the meantime, and I'm definitely open to other suggestions. > 3) src/backend/commands/extension.c > > I'm not sure why AlterExtensionNamespace moves the privilege check. Why > should it not check the privilege for owned schema too? Because for an owned schema we're not creating objects in an existing schema. We're only renaming the schema that the extension is already in using RenameSchema, so there's no point in checking if the user can create objects in that schema, since the objects are already there. (RenameSchema also checks internally if the current user is the owner of the schema) > Shouldn't binary_upgrade_extension_member still set ext=NULL in the for > loop, the way the original code does that? I don't think that's necessary. It was necessary before to set extobj to NULL, because that variable was set every loop. But now extobj is scoped to the loop, and ext is only set right before the break. So either we set ext in the loop and break out of the loop right away, or ext is still set to the NULL value that's was assigned at the top of the function. > The long if conditions might need some indentation, I guess. pgindent > leaves them like this, but 100 columns seems a bit too much. I'd do a > line break after each condition, I guess. Done
v4-0001-Add-support-for-extensions-with-an-owned-schema.patch
Description: Binary data
