Re: sql_drop Event Triggerg

On Fri, Mar 8, 2013 at 9:18 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> Robert Haas escribió:
>> On Tue, Mar 5, 2013 at 12:45 PM, Alvaro Herrera
>> <alvherre(at)2ndquadrant(dot)com> wrote:
>> > Hmm, maybe I should be considering a pair of macros instead --
>> > UTILITY_START_DROP and UTILITY_END_DROP. I'll give this a try. Other
>> > ideas are welcome.
>>
>> That seems like a possibly promising idea. I do wonder how well any
>> of this is going to scale.
>
> I did followup with a patch implementing that; did you see it?

No, sorry. Which thread is it on?

>> Presumably people are going to want
>> similar things for CREATE and (hardest) ALTER. Seems like
>> ProcessUtility() could get pretty messy and confusing. But I don't
>> have a better idea, either. :-(
>
> Well, the first thing that we need to settle is the user interface.
> Normalized command string don't seem to cut it; requiring users to write
> SQL parsers is rather unfriendly IMHO.

I could not agree more. The format we're moving towards for dropped
objects can easily be converted back into SQL if you happen to want to
do that, but it's also much easier to process programatically than a
compared with a normalized command string. So I think we get the best
of both worlds with this design.

> The current idea of having a
> function that returns objects affected by the command seems relatively
> sensible. For drops, it seems pretty straighforward so far. For CREATE
> it's probably somewhat more involved, but seems doable in principle (but
> yes, we're going to have to sprinkle ProcessUtility() with a lot of
> UTILITY_START/END_CREATE calls).
>
> Not sure about ALTER; maybe we will need a completely different idea to
> attack that.

I am inclined to think that putting this logic in ProcessUtility isn't
scalable, even for CREATE, and even moreso for ALTER, unless we can
put it around everything in that function, rather than each command
individually. Suppose for example that on entry to that function we
simply did this:

if (isCompleteQuery)
++CompleteQueryNestingLevel;

...and at exit, we did the reverse. This could work a bit like the
GUC nesting level. When an object is dropped, we find the array slot
for the current nesting level, and it's, say, a List **, and we push a
new element onto that list. When somebody asks for a list of dropped
objects, we pull from the list for the current nesting level. When
decrementing the nesting level, we flush the list associated with the
old nesting level, if any.

if (isCompleteQuery)
{
list_free(dropped_objects_list[CompleteQueryNestingLevel];
dropped_objects_list[CompleteQueryNestingLevel] = NIL;
--CompleteQueryNestingLevel;
}

Now, if we want to support CREATE, we can just have a
created_objects_list array as well, and only minimal changes are
required here. If we want to support ALTER we can have an
altered_objects_list, and the only decision is what data to stuff into
the list elements.

I think this is a lot better than decorating each individual command,
which is already unweildly and bound to get worse. It is a bit of a
definitional change, because it implies that the list of dropped
objects is the list of objects dropped by the most-nearly-enclosing
DDL command, rather than the list of objects dropped by the
most-nearly-enclosing DDL command *that is capable of dropping
objects*. However, I'm inclined to think that's a better definition
anyway.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company