Another extensions bug

Whilst testing the schema-creation-permissions scenario that Kaigai-san
recently pointed out, I happened to do this:

regression=# create schema c;
CREATE SCHEMA
regression=# create extension cube with schema c;
CREATE EXTENSION
regression=# drop schema c;
DROP SCHEMA

... er, what? I was expecting

ERROR: cannot drop schema c because other objects depend on it
DETAIL: extension cube depends on schema c
HINT: Use DROP ... CASCADE to drop the dependent objects too.

Inspection of pg_depend proved that indeed there was only a NORMAL
dependency of the extension on the schema, so this should not have
happened.

After a good bit of careful tracing, I came to the conclusion that the
bug is here in findDependentObjects, when it comes across an INTERNAL or
EXTENSION dependency in its first loop:

/*
* 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.
*/
findDependentObjects(&otherObject,
flags,
stack,
targetObjects,
pendingObjects,
depRel);

What happens in the cube case is that there's an INTERNAL dependency
from type cube[] to type cube, while both of these types also have
EXTENSION dependencies on the cube extension. So, when we trace the
INTERNAL dependency from type cube to type cube[], findDependentObjects
is entered with target object cube[] and flags = DEPFLAG_INTERNAL. In
its first loop, it discovers that cube[] has an EXTENSION dependency on
the cube extension, so the above code recurses to the extension object
with flags = DEPFLAG_INTERNAL. And that causes the extension to get
marked as being internally dependent on some other object-to-be-deleted,
which then licenses the dependency code to delete it silently and
without requiring CASCADE. Ooops.

I think the above-quoted code is just fundamentally wrong. It was never
possible to reach that code with flags = DEPFLAG_INTERNAL before,
because an object can have only one INTERNAL dependency and it's the one
that would have been caught by the preceding test to see if we were
recursing from the other end of that dependency. Now that we've added
the possibility of EXTENSION dependencies, we could get here for an
EXTENSION dependency after recursing from an INTERNAL dependency (as in
this case), or for an INTERNAL dependency after recursing from an
EXTENSION dependency. In neither of those cases does it make sense to
propagate the previous flags state. It is conceivable that we should
allow AUTO dependencies to propagate here, but I'm unconvinced; I don't
think the case can ever arise at present, and it certainly couldn't have
arisen before (there is no case where the system will label an object
with both AUTO and INTERNAL dependencies). If there is a reason to do
an AUTO drop of the referenced object, it should show up in some other
dependency of that object. So I'm thinking this recursive call should
just pass DEPFLAG_NORMAL in all cases:

findDependentObjects(&otherObject,
DEPFLAG_NORMAL,
stack,
targetObjects,
pendingObjects,
depRel);

I've not tested this heavily, but it fixes the example above and it
passes the regression tests.

Comments?

regards, tom lane