BUG #6172: DROP EXTENSION error without CASCADE

The following bug has been logged online:

Bug reference: 6172
Logged by: Hitoshi Harada
Email address: umi(dot)tanuki(at)gmail(dot)com
PostgreSQL version: 9.1RC1
Operating system: Windows7
Description: DROP EXTENSION error without CASCADE
Details:

On pure-installed RC1 database, you can CREATE EXTENSION, but cannot DROP
it.

CREATE EXTENSION cube;
DROP EXTENSION cube;

ERROR: cannot drop extension cube because other objects depend on it
DETAIL: operator <>(cube,cube) depends on function cube_ne(cube,cube)
operator >(cube,cube) depends on function cube_gt(cube,cube)
operator <=(cube,cube) depends on function cube_le(cube,cube)
operator >=(cube,cube) depends on function cube_ge(cube,cube)
operator <@(cube,cube) depends on function cube_contained(cube,cube)
operator ~(cube,cube) depends on function cube_contained(cube,cube)
HINT: Use DROP ... CASCADE to drop the dependent objects too.

DROP EXTENSION ... CASCADE; works but without CASCADE should work as source
build on Linux does it.

"Hitoshi Harada" <umi(dot)tanuki(at)gmail(dot)com> writes:
> On pure-installed RC1 database, you can CREATE EXTENSION, but cannot DROP
> it.
>
> CREATE EXTENSION cube;
> DROP EXTENSION cube;
>
> ERROR: cannot drop extension cube because other objects depend on it

I confirm I have the same bug in current HEAD.

Reading the code, my gut feeling is that the bug sits in
findDependentObjects(), in this part of it:

/*
* Okay, recurse to the other object instead of proceeding. We
* treat this exactly as if the original reference had linked
* to that object instead of this one; hence, pass through the
* same flags and stack.
*/

The extension cube depends on some operator that depend on some function
implementing them, and as the initial dependency delete call is not
explicitly mentioning them, then it behaves as if CASCADE was needed.

Also, \dx+ cube will not show all the operators and functions. It skips
those that we see in the CASCADE error message listing. Here's the SQL
query that will list the object with a direct extension dependency
towards the extension, here of OID 16385.

dim=# SELECT pg_catalog.pg_describe_object(classid, objid, 0) AS "Object Description"
dim-# FROM pg_catalog.pg_depend
dim-# WHERE refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass AND refobjid = '16385' AND deptype = 'e'
dim-# ORDER BY 1;

Of course we didn't have that problem when we added extensions in (that
I remember of), so I'm now going to try and find when that did change…

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> Also, \dx+ cube will not show all the operators and functions.

Some dependency information is indeed missing in pg_depend. Will look
at why tomorrow, day's over here.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
>> Also, \dx+ cube will not show all the operators and functions.

> Some dependency information is indeed missing in pg_depend. Will look
> at why tomorrow, day's over here.

I'm betting it's got something to do with
http://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=eb15f26d577a11319b9429fb84f752a0135918db

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> I'm betting it's got something to do with
> http://git.postgresql.org/gitweb/?p=postgresql.git&a=commitdiff&h=eb15f26d577a11319b9429fb84f752a0135918db

You're right, once more. Here's what I understand is happening from
reading the code. No patch attached, the scope of change I did is not
calling for one. I include full analysis in case you want to fix it in
another way, I could have missed something important here.

For reference, the error we're tracking begins with:

ERROR: cannot drop extension cube because other objects depend on it
DETAIL: operator <>(cube,cube) depends on function cube_ne(cube,cube)

The problem is that the following SQL will create the <> operator as a
Shell Operator then complete its definition later.

CREATE OPERATOR = (
LEFTARG = cube, RIGHTARG = cube, PROCEDURE = cube_eq,
COMMUTATOR = '=', NEGATOR = '<>',
RESTRICT = eqsel, JOIN = eqjoinsel,
MERGES
);

Here it's quite obvious that the '<>' operator (created as a shell) is
missing the dependency:

~:54320=# select oid, oprname from pg_operator
where oprleft = 'cube'::regtype and oprright = 'cube'::regtype and oprname in ('=', '<>');
oid | oprname
-------+---------
16421 | <>
16422 | =
(2 rows)

~:54320=# select * from pg_depend
where classid = 'pg_operator'::regclass and objid in (16421, 16422);
classid | objid | objsubid | refclassid | refobjid | refobjsubid | deptype
---------+-------+----------+------------+----------+-------------+---------
2617 | 16421 | 0 | 1255 | 16393 | 0 | n
2617 | 16421 | 0 | 1247 | 16386 | 0 | n
2617 | 16421 | 0 | 1247 | 16386 | 0 | n
2617 | 16421 | 0 | 2615 | 2200 | 0 | n

2617 | 16422 | 0 | 3079 | 16385 | 0 | e
2617 | 16422 | 0 | 1255 | 16392 | 0 | n
2617 | 16422 | 0 | 1247 | 16386 | 0 | n
2617 | 16422 | 0 | 1247 | 16386 | 0 | n
2617 | 16422 | 0 | 2615 | 2200 | 0 | n
(9 rows)

The code in pg_operator.c records the dependency on the Extension both
for a shell operator (in OperatorShellMake) and for a complete operator,
in OperatorCreate.

But in makeOperatorDependencies() we find the following piece of code:

/* In case we are updating a shell, delete any existing entries */
deleteDependencyRecordsFor(myself.classId, myself.objectId, false);

false is for bool skipExtensionDeps.

And now at the end of the same function, dependency is recorded back,
except in some case:

oldext = getExtensionOfObject(object->classId, object->objectId);
if (OidIsValid(oldext))
{
/* If already a member of this extension, nothing to do */
if (oldext == CurrentExtensionObject)
return;

The problem lies in catalog scans and SnapshotNow, I think. My fix is
to have deleteDependencyRecordsFor use true for skipExtensionDeps. Then:

~:54320=# drop extension cube;
DROP EXTENSION

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> But in makeOperatorDependencies() we find the following piece of code:

> /* In case we are updating a shell, delete any existing entries */
> deleteDependencyRecordsFor(myself.classId, myself.objectId, false);

> false is for bool skipExtensionDeps.

> And now at the end of the same function, dependency is recorded back,
> except in some case:

> oldext = getExtensionOfObject(object->classId, object->objectId);
> if (OidIsValid(oldext))
> {
> /* If already a member of this extension, nothing to do */
> if (oldext == CurrentExtensionObject)
> return;

> The problem lies in catalog scans and SnapshotNow, I think.

[ light goes on... ] We need a CommandCounterIncrement in there.
Else, the code that looks to see if the object is already part of the
extension does not see the pg_depend row as deleted (yet).

Not sure offhand where the cleanest place to put it is.

regards, tom lane

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> ... My fix is
> to have deleteDependencyRecordsFor use true for skipExtensionDeps.

On further reflection, there's some merit in that fix too. The question
is what do we think should happen if the pre-existing shell operator
belongs to another extension. It seems like the reasonable alternatives
are

(1) delete those pg_depend entries and allow the current extension to
take ownership.

(2) throw an error.

What you suggest above would result in (2), whereas what I was thinking
of would result in (1).

The case where this would actually happen is where extension A creates
some operator, and mentions some other operator as its commutator or
negator, but never gets around to defining the other operator. Then
extension B comes along and tries to fill in the other operator
definition. Do we want to let that happen, or do we want to throw an
error on the grounds that this sort of interconnection of two extensions
was almost certainly not intended? (Note that I rather doubt that
dropping either extension alone, afterwards, would clean up nicely,
since we have no code that would remove the oprcom/oprnegate linkage.)

A completely different line of thought is that maybe extension
membership records shouldn't be created at all for a shell operator,
on the grounds that it's not a real object but only a placeholder until
it's filled in.

One somewhat analogous situation is where we create a shell type and
then fill it in later. That code uses skipExtensionDeps = true and
so will end up throwing an error if the pre-existing membership is for
another extension. However, it's pretty hard to imagine a useful
situation where an extension would create a shell type and not fill it
in, so I'm not sure that this is a close analogy.

On the whole I'm starting to think that throwing an error is the best
thing. We could always relax that later, but going the other way might
be problematic.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> The case where this would actually happen is where extension A creates
> some operator, and mentions some other operator as its commutator or
> negator, but never gets around to defining the other operator. Then
> extension B comes along and tries to fill in the other operator
> definition. Do we want to let that happen, or do we want to throw an
> error on the grounds that this sort of interconnection of two extensions
> was almost certainly not intended? (Note that I rather doubt that
> dropping either extension alone, afterwards, would clean up nicely,
> since we have no code that would remove the oprcom/oprnegate linkage.)

I don't think we should let that happen. We currently support self
contained extensions and I don't see opening the door this way as a
feature.

> On the whole I'm starting to think that throwing an error is the best
> thing. We could always relax that later, but going the other way might
> be problematic.

+1

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support