Re: Command Triggers patch v18

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I've said repeatedly and over a long period of time that development
> of this feature wasn't started early enough in the cycle to get it
> finished in time for 9.2. I think that I've identified some pretty

That could well be, yeah.

> serious issues in the discussion we've had so far, especially (1) the
> points at which figures are fired aren't consistent between commands,
> (2) not much thought has been given to what happens if, say, a DDL
> trigger performs a DDL operation on the table the outer DDL command is
> due to modify, and (3) we are eventually going to want to trap a much
> richer set of events than can be captured by the words "before" and
> "after". Now, you could view this as me throwing up roadblocks to
> validate my previously-expressed opinion that this wasn't going to get
> done, but I really, honestly believe that these are important issues
> and that getting them right is more important than getting something
> done now.

(1) is not hard to fix as soon as we set a policy, which the most
pressing thing we need to do in my mind, whatever we manage to
commit in 9.2.

(2) can be addressed with a simple blacklist that would be set just
before calling user defined code, and cleaned when done running it.
When in place the blacklist lookup is easy to implement in a central
place (utility.c or cmdtrigger.c) and ereport() when the current
command is in the blacklist.

e.g. alter table would blacklist alter table and drop table commands
on the current object id

(3) we need to continue designing that, yes. I think we can have a first
set of events defined now, even if we don't implement support for
all of them readily.

> If we still want to try to squeeze something into 9.2, I recommend
> stripping out everything except for what Dimitri called the
> before-any-command firing point. In other words, add a way to run a

I would like that we can make that consistent rather than throw it, or
maybe salvage a part of the command we support here. It's easy enough if
boresome to document which commands are supported in which event timing.

> procedure after parsing of a command but before any name lookups have
> been done, any permissions checks have been done, or any locks have
> been taken. The usefulness of such a hook is of course limited but it
> is also a lot less invasive than the patch I recently reviewed and
> probably a lot safer. I actually think it's wise to do that as a
> first step even if it doesn't make 9.2, because it is much easier to
> build features like this incrementally and even a patch that does that
> will be reasonably complicated and difficult to review.

Yes.

> Parenthetically, what Dimitri previously called the after-any-command
> firing point, all the way at the end of the statement but without any
> specific details about the object the statement operated on, seems
> just as good for a first step, maybe better, so that would be a fine
> foundation from my point of view as well. The stuff that happens

Now that fetching the information is implemented, I guess that we could
still provide for it when firing event trigger at that timing spec. Of
course that means a bigger patch to review when compared to not having
the feature, but Thom did spend loads of time to test this part of the
implementation.

> somewhere in the middle, even just after locking and permissions
> checking, is more complex and I think that should be phase 2
> regardless of which phase ends up in which release cycle.

Mmmm, ok.

> I'm not sure whether Dimitri's question about 9.3 was a typo for 9.2

Typo :) I appreciate your offer, though :)

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