Re: Command Triggers

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.

That would require an hook registration API, and would allow a much
easier security auditing of a setup you don't know beforehand.

So I think that hooks not being documented is an implementation detail
here :)

> 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.

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.

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.

> 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. So now something

Andres has been sending a complete patch to fix that old oddity of the
parser. And again, that's a very narrow exception to the usual state of
things, and as such, easy to document in the list of things that don't
fall into the general rule.

Even when fixed, though, you have 3 different command tags to deal with,
and we identify the command triggers on command tags, so that a command
trigger on CREATE TABLE will not be called when doing SELECT INTO, nor
when doing CREATE TABLE AS.

Also, I think that the command triggers in 9.2 will not address all and
any command that PostgreSQL offers (think “alter operator family”), so
that we will need to maintain an exhaustive list of supported commands,
identified by command tags.

> 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 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.

And we don't have to do that unless we want to make subcommands
available to ProcessUtility_hook and command triggers.

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