Re: BUG #6704: ALTER EXTENSION postgis SET SCHEMA leaves dangling relations

Excerpts from Tom Lane's message of vie ago 31 17:50:41 -0400 2012:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:

> > 2. During ALTER EXTENSION execution, skip moving objects that have
> > already been moved. Not really sure how this would be implemented;
>
> +1 for this approach. I'm a bit surprised we didn't hit this before,
> because in general there can be multiple dependency chains leading from
> object A to object B. Most code that is doing more than trivial
> dependency-walking has to be prepared to cope with reaching the same
> object multiple times.
>
> Implementation like this seems reasonable:
>
> > 4. Maybe we could have AlterRelationNamespaceInternal check what the
> > current namespace is for the object, and do nothing if it's already the
> > target namespace.
>
> We already have some such shortcut for ALTER OWNER, IIRC, so why not
> for SET SCHEMA as well? I suspect that AlterRelationNamespaceInternal
> is not the only function that needs it, too.

It doesn't work :-( The problem is that the outer sysscan in
extension.c gets to the table first, recurses there and updates the
sequence pg_depend tuple; then it gets out and the outer scan gets to
the sequence directly. But this fails to notice that it has already
been updated, because we haven't done a CommandCounterIncrement.
However, if I add one, we get into Halloween problem because the
sequence is updated, command counter incremented, and the outer scan
sees the updated tuple (because it's using SnapshotNow) for the table so
it recurses again, and instead of "tuple updated by self" we get this:

alvherre=# alter extension isn set schema baz;
ERROR: relation "test_b_seq" already exists in schema "baz"

So I think my other proposal is the way to fix the problem: each
AlterFooNamespace routine must update an ObjectAddresses array of
relocated objects, and the outer scan (extension.c) must skip objects
that are already in that list.

I have tested this theory (attached patch) and it solves the problem at
hand. The patch is not complete: I haven't updated all the
AlterFooNamespace routines, only those necessary to fix this problem.
If we agree that this is the way to go I can complete and push.

Putting this kind of change this late in the 9.2 release process makes
me a bit nervous, but I don't see a simpler way to solve the problem.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services