Re: Command Triggers

On Thursday, December 15, 2011 04:53:15 PM Dimitri Fontaine wrote:
> Hi,
>
> Thank your Robert for this continued review, I think we're making good
> progress in a community discussion that needs to happen!
>
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >> 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.

> Mmmm, yes. I think that we should document hooks, and I'm going to
> propose soon a pg_extension_module() SRF that lists all currently loaded
> modules and which extension implements them, and I've begun thinking
> about what it would take to be able to list what hooks each module is
> implementing.
I don't really see that as possible/realistic.

> > 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
>
> Yes, this CREATE SCHEMA <any create command> is weird, and unique, so
> it's not that difficult to document, I think. And if we fix things by
> having all subcommands go through ProcessUtility and command triggers,
> then it's easy to document as the new rule.
I don't think I ever saw that one in real worldddl scripts ;)

> My documentation idea would then be explaining what is a command and
> what is a subcommand, and then the current rule (command triggers do not
> support subcommands) and then the exception to that rule (CREATE SCHEMA
> subcommands do, in fact, support command triggers).

> If we decide that we should in fact support (nested) subcommands in
> command triggers, then we will be in a position to prepare a patch
> implementing that with a documentation change that the rule is now that
> command triggers do in fact support subcommands.

> When the command trigger is called on a subcommand, I think you will
> want both the main command string and the subcommand string, and we have
> to research if what you need isn't a whole stack of commands, because
> I'm not sure you can only have 1 level deep nesting here.
I don't think you need that. If needed you will have to build the data
structure in $pl.

> That would be done with a special API for the trigger functions, and
> with a specific syntax, because it seems to me that having a trigger
> code that is only ever called on a top-level command is a good thing to
> have.
>
> create trigger foo after command CREATE TABLE …
> create trigger foo after top level command CREATE TABLE …
> create trigger foo after nested command CREATE TABLE …
>
> Either we add support for that kind of syntax now (and not implementing
> it in 9.3 would seem weird as hell) or we instruct pg_dump and
> pg_upgrade to switch from current syntax to the new one (add in the “top
> level” keywords) when we do implement the feature down the road.
I personally think there should only be one variant which is always called.
With a parameter that indicates whether its a subcommand or not.

Why would you ever only want top level commands?

> > 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?
> Yes, we need to make a decision about that now. Do we want any
> “operation” to go through ProcessUtility so that hooks and command
> triggers can get called?
I personally think thats a good idea for most stuff.

I don't see that for alter table subcommands and such though.

> I think it's a good idea in the long run, and Alvaro seems to be
> thinking it is too. That entails changing the backend code to build a
> Statement then call ProcessUtility on it rather than calling the
> internal functions directly, and that also means some more DDL are
> subject to being logged on the server logs. Removing those
> “cross-modules” #include might be a good think in the long run though.
Uhm. I don't think building strings is the way to go here. I think building
*Stmt nodes is better.

Andres