Alvaro Herrera <alvherre@commandprompt.com> writes:
> Alex Hunsaker escribi�:
>> I'm not proposing this patch for actual submission, more of a would this work?
>> If I'm not missing something glaring obvious Ill go ahead and make the
>> rest of the Remove things behave the same way
> I don't think there's anything wrong with that in principle. However,
> does your patch actually work? The changes in expected/ is unexpected,
> I think.
No, they look right --- we're losing gripes about earlier tables
cascading to subobjects of later tables, which is exactly what we want.
I don't really like the patch though; it seems kind of a brute force
solution. You've got ProcessUtility iterating through a list of objects
and doing a little bit of work on each one, and then making a new list
that RemoveRelation (which is now misnamed) again iterates through and
does a little bit of work on each one, and then passes that off *again*.
There's no principled division of labor at work there; in particular
it's far from obvious where things get locked. You've also managed
to embed an assumption not previously present, that all the objects in
the list are of exactly the same type.
I think it would be better if the DropStmt loop were responsible
for constructing a list of ObjectAddresses that it handed off directly
to performMultipleDeletions. This is why I was imagining replacing
RemoveRelation et al with something that passed back an ObjectAddress,
and getting worried about introducing references to ObjectAddress into
all those header files. Another possibility would be to get rid of
RemoveRelation et al altogether and embed that code right into the
DropStmt processing (though at this point we'd need to split it out
of ProcessUtility; it's already a bit large for where it is). That
didn't seem tremendously clean either, though it would at least have
the virtue of improving consistency about where privilege checks etc
get done.
regards, tom lane