Обсуждение: DROP OWNED BY doesn't work

So I was fooling with making serial sequences be auto rather than internal
dependencies of their columns, and the regression tests blew up on me:

*** ./expected/dependency.out    Mon Nov 21 07:49:33 2005
--- ./results/dependency.out    Sat Aug 19 17:46:55 2006
***************
*** 109,113 ****
--- 109,117 ---- DROP USER regression_user2; ERROR:  role "regression_user2" cannot be dropped because some objects
dependon it DROP OWNED BY regression_user2, regression_user0;
 
+ NOTICE:  default for table deptest column a depends on sequence deptest_a_seq
+ ERROR:  cannot drop sequence deptest_a_seq because other objects depend on it DROP USER regression_user2;
+ ERROR:  role "regression_user2" cannot be dropped because some objects depend on it DROP USER regression_user0;
+ ERROR:  role "regression_user0" cannot be dropped because some objects depend on it

On investigation I find that DROP OWNED BY tries to drop objects in the
more-or-less-random order that they appear in pg_shdepend.  This doesn't
work, at least not without CASCADE.  I think people should be able to
assume that DROP OWNED BY RESTRICT will drop all the target users'
objects so long as there are not objects owned by other users that
depend on them.

There's a hack in there now that tries to special-case this for INTERNAL
dependencies, but I think it's misguided.  You can run into the problem
without any internal or auto dependencies:

regression=# create user foo;
CREATE ROLE
regression=# \c - foo
You are now connected to database "regression" as user "foo".
regression=> create table tt1 (f1 int);
CREATE TABLE
regression=> create table tt2 (f1 int primary key);
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "tt2_pkey" for table "tt2"
CREATE TABLE
regression=> alter table tt1 add foreign key (f1) references tt2;
ALTER TABLE
regression=> drop owned by foo;
NOTICE:  constraint tt1_f1_fkey on table tt1 depends on table tt2
ERROR:  cannot drop table tt2 because other objects depend on it
HINT:  Use DROP ... CASCADE to drop the dependent objects too.
regression=> 

I think a correct solution probably requires making a list of all
objects to delete by scanning pg_shdepend and then starting to
delete 'em, using the list as "oktodelete" context similar to the
way that dependency.c handles auto/internal objects.
        regards, tom lane

Tom Lane wrote:

> I think a correct solution probably requires making a list of all
> objects to delete by scanning pg_shdepend and then starting to
> delete 'em, using the list as "oktodelete" context similar to the
> way that dependency.c handles auto/internal objects.

Hum.  I'll take a look at doing that.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Tom Lane wrote:

> I think a correct solution probably requires making a list of all
> objects to delete by scanning pg_shdepend and then starting to
> delete 'em, using the list as "oktodelete" context similar to the
> way that dependency.c handles auto/internal objects.

What I'm considering is this: scan pg_shdepend looking for objects to
delete, and save them into a list; but each time we find one, we also
find objects that depend on it.  Those dependent objects should be
ignored; but we should also remove from the list of objects to delete,
any dependent object that we added in a previous iteration of the
pg_shdepend scan.  A subtlety is that if we see that an object from the
scan is in the ignored list, we need not find dependent objects nor add
it to the to-delete list.


In pseudo-code it would be something like this:

ObjectAddresses objects-to-delete = empty
ObjectAddresses objects-ignored = empty

scan pg_shdepend for objects depending on the user;
for (object) in scanif (object is not in objects-ignored)    ObjectAddresses dependent-objects = empty
    add object to objects-to-delete    dependent-objects = findAutoDeletableObjects(object)
    add dependent-objects to objects-ignored    for (object2) in objects-to-delete        if (object2 is in
objects-ignored)           remove object2 from objects-to-delete            add object2 fo objects-ignored
 

for (object) in objects-to-deleteperformDeletion(object)


This has a nasty looking O(n^2) behavior, but I don't see anything
better.

Comments?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre@commandprompt.com> writes:
> What I'm considering is this: scan pg_shdepend looking for objects to
> delete, and save them into a list; but each time we find one, we also
> find objects that depend on it.  Those dependent objects should be
> ignored; but we should also remove from the list of objects to delete,
> any dependent object that we added in a previous iteration of the
> pg_shdepend scan.

Um ... but this doesn't seem to acknowledge the idea that we should
distinguish whether those dependent objects are owned by the user or
not.

What I was thinking of was
find all target objects using pg_shdepend, make an    ObjectAddresses list of them
for (each item of list){    if (already deleted)        continue;    if (implicit dependency of some later list item)
    continue;    pass item to performDeletion, with entire list        as oktodelete context}
 

This will require some cooperation from dependency.c: the standard
performDeletion entry point doesn't let you pass in an oktodelete list,
and it would be nice if "performDeletionWithList" or whatever we
call it provided some feedback about what it'd deleted so that the
bookkeeping effort implicit in "if (already deleted)" could be
minimized.

> This has a nasty looking O(n^2) behavior, but I don't see anything
> better.

In my formulation the O(N^2) behaviors are limited to searching the
oktodelete list, which at least is a pretty tight inner loop.  The
dependency.c code was not designed with the idea that oktodelete would
ever get really large ... perhaps later we could revisit that data
structure, but for now I think this'll be good enough.
        regards, tom lane

I found one other problem in this area, which was that REASSIGN OWNED
didn't work real well either after I changed serial sequences'
dependency type to AUTO.  What I did about it was to make
shdepReassignOwned call ATExecChangeOwner with recursing = true,
which suppresses all those tedious error checks ;-).  This means we
don't need objectIsInternalDependency() at all anymore: just barrel
ahead and try to change owner on everything.
        regards, tom lane