ALTER TYPE vs extension membership (was Re: BUG #17144: Upgrade from v13 to v14 with the cube extension failed)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: a(dot)kozhemyakin(at)postgrespro(dot)ru
Subject: ALTER TYPE vs extension membership (was Re: BUG #17144: Upgrade from v13 to v14 with the cube extension failed)
Date: 2021-08-16 19:36:59
Message-ID: 985050.1629142619@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

I wrote:
> Hm, thanks. This does not seem to be a problem with pg_upgrade per se;
> you can reproduce it with

> regression=# CREATE EXTENSION cube VERSION '1.4';
> CREATE EXTENSION
> regression=# CREATE EXTENSION earthdistance;
> CREATE EXTENSION
> regression=# ALTER EXTENSION "cube" UPDATE;
> ERROR: type earth is already a member of extension "earthdistance"

> [ experiments a bit more ] It might just be directly-dependent types.
> If I create a table column of type cube, then nothing strange happens
> during the upgrade. But if I create a domain over cube, then do the
> update, the domain gets absorbed into the extension. That'd be kind
> of annoying :-(

So the problem is that ALTER TYPE SET recurses to dependent domains,
as it must, but it is not careful about what that implies for extension
membership. It looks like we need to extend GenerateTypeDependencies
so that it knows not to mess with extension dependencies when doing
an ALTER.

There's a policy question here, which is when does an operation on
a pre-existing object within an extension script cause the object
to be absorbed into the extension? You might naively say "never",
but that's not our historical behavior, and I think it'd clearly
be the wrong thing in some cases. For example, consider

CREATE TYPE cube; -- make a shell type
-- do something that requires type cube to exist
CREATE EXTENSION cube;

We don't want this to fail, because it might be necessary to do
things that way to get out of circular dependencies. On the other
hand, the formerly-shell type had certainly better wind up owned
by the extension.

The general policy as the code stands seems to be that CREATE OR
REPLACE-style operations will absorb any replaced object into
the extension. IMO extension scripts generally shouldn't use
CREATE OR REPLACE unless they're sure they already have such an
object; but if one does use such a command, I think this behavior
is reasonable.

The situation is a lot less clear-cut for ALTER commands, though.
Again, it's dubious that an extension should ever apply ALTER to
an object that it doesn't know it already owns; but if it does,
should that result in absorbing the object? I'm inclined to think
not, so the attached patch just causes AlterTypeRecurse and
AlterDomainDefault to never change the object's extension membership.
That's more behavioral change than is strictly needed to fix the
bug at hand, but I think it's a consistent definition.

I looked around for other places that might have similar issues,
and the only one I could find (accepting that CREATE OR REPLACE
should work this way) is that ALTER OPERATOR ... SET applies
makeOperatorDependencies, which has the same sort of behavior as
GenerateTypeDependencies does. I'm inclined to think that for
consistency, we should make that case likewise not change extension
membership; but I've not done anything about it in the attached.

Another point that perhaps deserves discussion is whether it's
okay to change the signature of GenerateTypeDependencies in
stable branches (we need to fix this in v13 not only v14/HEAD).
I can't see a good reason for extensions to be calling that,
and codesearch.debian.net knows of no outside callers, so I'm
inclined to just change it. If anyone thinks that's too risky,
we could do something with a wrapper function in v13.

Comments?

regards, tom lane

Attachment Content-Type Size
dont-change-extension-membership-in-ALTER-TYPE.patch text/x-diff 5.2 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2021-08-17 04:34:28 BUG #17148: About --no-strict-names option and --quiet option of pg_amcheck command
Previous Message Chuck Nellis 2021-08-16 16:38:46 RE: Re: psql crash when running a procedure with an inout parameter and a commit

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2021-08-16 19:38:42 Re: when the startup process doesn't (logging startup delays)
Previous Message Tom Lane 2021-08-16 19:09:03 Re: [PATCH] Native spinlock support on RISC-V