Re: Event Triggers: adding information

Robert Haas <robertmhaas@gmail.com> writes:
> Goal: Every time an ALTER command is used on object *that actually
> exists*, we will call some user-defined function and pass the object
> type, the OID of the object, and some details about what sort of
> alteration the user has requested.

Ok, in current terms of the proposed patch, it's a ddl_command_end event
trigger, and you can choose when to fire it in utility.c. The current
place is choosen to be just after the AlterTable() call, because that
sounded logical if you believe in CommandCounterIncrement.

> So, instead, what we need to do is go into each function that
> implements ALTER, and modify it so that after it does the dance where
> we check permissions, take locks, and look up names, we call the
> trigger function.  That's a bit of a nuisance, because we probably
> have a bunch of call sites to go fix, but in theory it should be
> doable.  The problem of course, is that it's not intrinsically safe to
> call user-defined code there.  If it drops the object and creates a
> new one, say, hilarity will probably ensue.

You're trying to find a dangerous point when to fire the trigger here,
rather than trying to solve the problem you're describing. For the
problem you're describing, either you want the Object Specific
Information and the trigger fires at ddl_command_end, or you don't and
you can register your trigger at ddl_command_start.

If you want something else, well, it won't ship in 9.3.

> Now, there is a further problem: all of the information about the
> ALTER statement that we want to provide to the trigger may not be
> available at that point.  Simple cases like ALTER .. RENAME won't
> require anything more than that, but things like ALTER TABLE .. ADD
> FOREIGN KEY get tricky, because while at this point we have a solid
> handle on the identity of the relation to which we're adding the
> constraint, we do NOT yet have knowledge of or a lock on the TARGET
> relation, whose OID could still change on us up to much later in the
> computation.  To get reliable information about *that*, we'll need to
> refactor the sequence of events inside ALTER TABLE.

The only valid answer here has already been given by Tom. You can only
provide the information if it's available in the catalogs. So again,
it's a ddl_command_end event. It can not happen before.

> Hopefully you can see what I'm driving toward here.  In a design like
> this, we can pretty much prove - after returning from the event
> trigger - that we're in a state no different from what would have been
> created by executing a series of commands - lock table, then SELECT
> event_trigger_func(), then the actual ALTER in sequence.  Maybe
> there's a flaw in that analysis - that's what patch review is for -
> but it sounds pretty darn tight to me.

The only case when we need to do a lookup BEFORE actually running the
command is when that command is a DROP, because that's the only timing
when the information we want is still in the catalogs.

So that's the only case where we do a double object lookup, and we take
a ShareUpdateExclusiveLock lock on the object when doing so, so that we
can't lose it from another concurrent activity.

> Note that the above is clearly
> not ddl_command_start but something else, and the different firing
> point and additional safety cross-checks are what enable us to pass
> additional information to the trigger without fear of breaking either
> the trigger or the world.

That's the reason why we are only going to have ddl_command_start and
ddl_command_end in 9.3, and also the reason why the extra information is
only provided when this very information still exists in the catalogs at
the moment when we want to expose it.

And that's the reason why ddl_command_trace is attractive too: you won't
ever get Object OID and Name and Schema from the event where the object
does not exists, and maybe you want an easy way to tell the system
you're interested into an event *with* the information, and be done,
don't think.

Again, I don't care much about keeping ddl_command_trace because it's
not interesting much for implementing DDL support in logical
replication and my other use cases, but still, I though I would explain
it now that we're talking about it.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support