Thread: Re: DROP OWNED BY doesn't work

Tom Lane wrote:

> 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
>     }

Ok, this patch implements more or less this idea; except that instead of
checking the list of implicit dependencies for later objects, we iterate
twice on the list of objects to delete, and create a list of implicit
dependencies on all of them the first time, and delete those not on that
list the second time.

Regression tests pass.  This patch makes the following example work as
expected.  Note that the first "drop owned" is stopped by the FK on tt3;
by adding the cascade, it drops the foreign key.

Let me know if it fixes your problem, and I'll commit it early tomorrow.
(Or you can do it if you're in a hurry ...)


create user foo;
create user bar;

\c - bar
create table tt3 (f1 int);

\c - foo
create table tt1 (f1 int);
create table tt2 (f1 int primary key);
alter table tt1 add foreign key (f1) references tt2;
grant references on table tt2 to bar;

\c - bar
alter table tt3 add foreign key (f1) references tt2;

\c - foo
drop owned by foo;
drop owned by foo cascade;



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

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Ok, this patch implements more or less this idea; except that instead of
> checking the list of implicit dependencies for later objects, we iterate
> twice on the list of objects to delete, and create a list of implicit
> dependencies on all of them the first time, and delete those not on that
> list the second time.

In the light of morning it occurs to me that probably the best code
factoring for this is for dependency.c to export a
performMultipleDeletions() function that takes an objectAddresses list
of targets and does all the dependency-chasing internally.  This would
reduce the amount of cruft that has to be exported from dependency.c.

Other than that, the patch looks plausible in a very fast scan.

            regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Ok, this patch implements more or less this idea; except that instead of
> > checking the list of implicit dependencies for later objects, we iterate
> > twice on the list of objects to delete, and create a list of implicit
> > dependencies on all of them the first time, and delete those not on that
> > list the second time.
>
> In the light of morning it occurs to me that probably the best code
> factoring for this is for dependency.c to export a
> performMultipleDeletions() function that takes an objectAddresses list
> of targets and does all the dependency-chasing internally.  This would
> reduce the amount of cruft that has to be exported from dependency.c.

Sounds a pretty obvious move.  This would be it then.  Please note the
changes to the ObjectAddresses stuff, mainly to not move the definition
of struct ObjectAddresses to dependency.h but instead move only the
typedef.

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

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Sounds a pretty obvious move.  This would be it then.  Please note the
> changes to the ObjectAddresses stuff, mainly to not move the definition
> of struct ObjectAddresses to dependency.h but instead move only the
> typedef.

Looks pretty reasonable, except if you're going to do the latter I'd
suggest just having new_object_addresses() and free_object_addresses()
and getting rid of the init/term API for having the structs on the
stack.  The latter is saving one palloc/pfree per deletion cycle, which
is a pretty silly micro-optimization really given everything else that's
going to happen to perform the delete.  It was OK when there wasn't any
reason to do otherwise, but I can't see maintaining it in parallel with a
version that does the extra palloc/pfree, and especially not adding a
bool to the struct to support having both ...

Please fix that bit and apply.

            regards, tom lane