Re: Event Triggers: adding information

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I think this points to a couple of problems: this patch isn't
> well-enough thought out, and it's got several features jammed into a
> single patch. This should really be split up into several patches and
> each one submitted separately.

Ok. Now I want to talk about our process a little. That's a 2 paragraphs
diversion, after that it's getting back to technical matters.

There's a difference between "it's not the way I would have done it" and
"the author didn't think about what he's doing". That's also the reason
why it's very hard to justify sending a polished enough patch as a non
commiter.

And then this patch is like the next one in a long series that is
lasting for about 2 years now, and spliting it is just more work for
everybody, and then you take the risk that the next commiter who looks
at the patch prefers to see a complete enough view of the goal you're
trying to reach.

> I think that ddl_command_trace is an unnecessary frammish that should
> be ripped out entirely. It is easy enough to accomplish the same
> thing with ddl_command_start and ddl_command_end, and so we're just
> increasing the number of cases we have to support in the future for no
> real benefit.

I think you might want to review the use case behing ddl_command_trace,
that has been asked by who users wanted to see their use case supported
in some easier way than just what you're talking about here.

> This gets right back to an argument Dimitri and I have been having
> since v1 of this patch, which is whether these are command triggers or
> event triggers. I think we ought to support both, but they are not
> the same thing. I think the design you are proposing is just fine for
> an event called sql_drop, but it makes no sense for an event called
> ddl_command_end, which needs to be called once per DDL statement - at
> the end. Not once per object dropped.

What I think you're missing here is the proposal flying around to have
drop operation get back to ProcessUtility so that C hooks and event
triggers both can have at it. It's been debated on the list earlier, way
before we talked about event triggers, also as a way to clean up things,
IIRC.

I agree that if we keep the code as it is to implement cascading
behavior, a sql_drop event makes more sense.

> TOPLEVEL is supposed to correspond to a complete SQL statement entered
> by the user:

Having the DROP command get back to ProcessUtility would allow us to
have the nested events as GENERATED, but CASCADE would allow an easier
way to just match that if that's what users are interested into.

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