Re: Another extensions bug

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> Hence, proposed patch attached (which also improves some of the related
>> comments).

> +1 on the idea, although I'm not in a position to further review or play
> with the patch today.

Further testing shows that this patch still doesn't make things really
work properly. There's yet *another* bug with extension-dropping: given
a case such as types cube[] and cube both belonging to extension cube,
the end result is that type cube[] never gets dropped at all! This is
because, since cube[] has both INTERNAL and EXTENSION dependencies, it's
impossible for findDependentObjects to get past its first loop when
cube[] is the target. It will always recurse to the other owning
object. Infinite recursion is avoided because it finds the other owning
object already on the stack, but we never get to the point of deciding
that it's okay to delete type cube[]. So:

test=# create extension cube;
CREATE EXTENSION
test=# \dT
List of data types
Schema | Name | Description

--------+------+----------------------------------------------------------------
-----------------------------
public | cube | multi-dimensional cube '(FLOAT-1, FLOAT-2, ..., FLOAT-N), (FLOA
T-1, FLOAT-2, ..., FLOAT-N)'
(1 row)

test=# drop extension cube;
DROP EXTENSION
test=# \dT
List of data types
Schema | Name | Description
--------+-------+-------------
public | ???[] |
(1 row)

test=#

This is another bug that was arguably latent in the original coding,
but could not manifest as long as there was only one INTERNAL-like
dependency per object.

I think the right fix is to replace the simple "is the referenced object
on top of the stack?" check at lines 600ff with a check for "is the
referenced object anywhere in the stack?".

It's also becoming brutally obvious that we need some regression tests
checking extension drop scenarios. I'll work on that today too.

regards, tom lane