Re: Command Triggers

On Thursday, December 15, 2011 03:36:25 PM Robert Haas wrote:
> On Wed, Dec 14, 2011 at 5:44 PM, Dimitri Fontaine
>
> <dimitri(at)2ndquadrant(dot)fr> wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >>> I can get behind this argument: force all stuff through ProcessUtility
> >>> for regularity, and not necessarily in the first patch for this
> >>> feature.
> >> That seems like a pretty heavy dependency on an implementation detail
> >> that we might want to change at some point.
> > Given ProcessUtility_hook, how much of an implementation detail rather
> > than a public API are we talking about?
> I think that a hook and an SQL command are not on the same level.
> Hooks are not documented; SQL commands are. You may, of course,
> disagree.
I am not disagreeing.

> But the basic point is that it seems pretty weird to say, on the one
> hand, these are command triggers. They fire when you execute a
> command. So the CREATE TABLE trigger will fire when someone says
> CREATE TABLE. But then we say, oh, well if you use CREATE SCHEMA foo
> CREATE TABLE blah ... we will fire the trigger anyway. Now it's not a
> command trigger any more; it's a trigger that fires when you perform a
> certain operation - e.g. create a table. Unless, of course, you
> create the table using CREATE TABLE AS SELECT or SELECT .. INTO. Then
> it doesn't fire. Unless we decide to make those utility commands,
> which I think was just recently under discussion, in which case it
> will suddenly start firing for those operations.
Command triggers were the initial reason for me doing that conversion.

> So now something
> that is, right now, an essentially an implementation detail which we
> can rearrange as we like turns into a user-visible behavior change.
> And what if some day we want to change it back?

> I think it would be a much better idea to decree from the beginning
> that we're trapping the *operation* of creating a table, rather than
> the *command* create table. Then, the behavior is clear and
> well-defined from day one, and it doesn't depend on how we happen to
> implement things internally right at the moment.
I am with you there.

> If there's some way
> of creating a table without firing the trigger, it's a bug. If
> there's some way of firing the trigger without attempting to create a
> table, it's a bug.
Again.

> That might require some more thought about what
> information to pass to the trigger function (e.g. if it's a SELECT ..
> INTO, you're not going to have pregenerated SQL that starts with the
> words "CREATE TABLE") but the fact that gives much more clear
> definition to the core feature seems like a big plus in my book.
Not sure what youre getting at here? The command tag in Dim's patch is alread
independent of the form of actual statement that was run.

A big +1 on the general direction of this email. I dislike reducing the scope
of command triggers to "top level commands run by the actual user" because imo
that would reduce the possible scope of the patch rather much.

On the other hand I think delivering a complete patch covering just about
anything triggered anywhere is not really realistic. Not sure whats the best
way to continue is.

Andres