Re: Command Triggers patch v18

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> How about calling it "command tag"? I think both context and toplevel
> are inconsistent with other uses of those terms: context is an
> error-reporting field, among other things; and we don't care about the
> toplevel command, just the command-tag of the one we're executing -
> e.g. if DROP fires a command trigger which invokes CREATE which fires
> another command trigger, the inner one is going to get CREATE not
> DROP. Or at least so I presume.

It's not about that though, it's about a DROP TYPE that cascades to a
DROP FUNCTION, or a DROP SCHEMA that cascades to 10 DROP TABLE. I want
to know in each cascaded DROP TABLE that it's happening as a result of
DROP SCHEMA ... CASCADE, so I'm calling that a top-level command.

> See above example: I am pretty sure you need a stack.

In next version, certainly. As of now I'm willing to start a new stack
in each command executed in a command trigger. That means 9.2 will only
expose the first level of the stack, I guess.

Also there's a difference in CASCADE (no new command emitted) and in an
event trigger that executes a new top-level command.

> I would not want my replication system issuing cascaded drops, because
> if the sides don't match it might cascade to something on the remote
> side that it doesn't cascade to on the local side, which exceeds my
> tolerance for scary behavior.

Well it depends on what you're achieving with replication, this term
includes so many different use cases… What I want core to provide is the
mechanism that allows implementing the replication you need.

>> There are far too many variants and cases of our command to be able to
>> extract their parameters in a flat way (a bunch of variables compared to
>> a nested description ala json or xml), and I don't think such a flat
>> representation is going to be much better than the parse tree.
>
> I strongly disagree. I think we'll find that with the right choice of
> hook points, the number of variables that need to be exposed is quite
> compact. Indeed, I'd venture to say that needing to pass lots and
> lots of information is evidence that you've made a poor choice of hook
> point.

Currently we're exposing a very limited set of variables. So I think
we're good in your book.

> No, but whether or not you mention it in the CREATE TRIGGER syntax has
> nothing to do with whether it's available as a magic parameter inside
> the procedure. Those things out to be independent. I imagine that
> the stuff that is accessible from inside the trigger will be richer
> than what you can do in the trigger syntax itself.

Exactly, we're in agreement here.

> I'm still not sold on the idea of lumping together every command under
> a single command_start event. I can't see anyone wanting to hook
> anything that broad. Don't forget that we need to document not only
> which triggers will fire but also what magic variables they'll get. A

Yeah, and command start triggers are only going to have tag, toplevel
tag or whatever the right name of that is, and parse tree if it's
written in C. And that's it. The command start event trigger are
typically fired directly from utility.c.

> dcl_command_start hook could conceivably get the list of privileges
> being granted or revoked, but a general command_start trigger is going
> to need a different set of magic variables depending on the actual
> command type. I think it might be simpler and more clear to say that
> each event type provides these variables, rather than having to
> conditionalize it based on the command type.

That's going to be true for other event timing specs, but not for the
command start as I picture it.

> See above - generally, I think that it's useful for a command trigger
> to know that it's being called because of a DDL event, rather than
> some command that could be doing anything. Also, I think that wanting
> to hook "all DDL commands" is likely to be a useful thing to do, and
> without having to explicitly list 50 command names...

Yeah, just omit the WHEN clause then.

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