Re: Command Triggers

From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Greg Smith <greg(at)2ndquadrant(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>
Subject: Re: Command Triggers
Date: 2012-02-07 12:51:33
Message-ID: CA+U5nMKSsByGBtj1dAXn5X-+oY77gDgdxTquJ0tB6xt+=OSWUQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 26, 2012 at 10:00 PM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
> Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
>>> Really I think there is not any single point where you can put the
>>> command-trigger hook and be done.  In almost every case, the right
>>> place is going to be buried somewhere within the execution of the
>>> command.
>>
>> I'm finally realizing it. I already introduced a structure called
>> CommandContextData to keep track of the current command elements we want
>> to pass down to command triggers, I think we're saying that this should
>> be a static variable that commands will need to be editing.
>
> In fact passing it as an argument to the command trigger API is much
> simpler and done in the attached. I'm having problems here with my
> install and not enough time this week (you don't speak English if you
> don't use understatements here and there, right?) so please expect a
> one-step-further patch to show the integration concept, not a ready for
> commit one just yet.
>
> Next steps are about adding support for more commands, and now that we
> have settled on a simpler integration that will be easy. The parameters
> sent to the trigger procedure are now the command tag, the main object
> Oid, its schema name and its object name. Only the command tag will
> never be NULL, all the other columns could be left out when calling an
> ANY COMMAND trigger, or a command on a schemaless object.
>
> Note: the per-command integration means the Oid is generally available,
> so let's just export it.
>
> An ack about the way it's now implemented would be awesome, and we could
> begin to start about which set of command exactly we want supported from
> the get go (default to all of them of course, but well, I don't think
> that's necessarily the best use of our time given our command list).

Looks good so far.

A user centric review of this patch...

Would like a way to say ALL COMMANDS rather than write out list of
supported commands. That list is presumably subject to change, so not
having this could result in bugs in later code.

The example given in the docs has no explanation. More examples and
explanation please. Would specifically be interested in a command
trigger which simply prevents all commands, which has many uses and is
simple enough to be in docs.

Why do we exclude SEQUENCEs? That could well be a problem for intended users

Please explain/add to docs/comments why ALTER VIEW is not supported?
Why not other commands??
Not a problem, but users will ask, so we should write down the answer

Please document why pg_cmdtriggers is a separate table and not
enhancement of pg_triggers?

Thanks

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chetan Suttraway 2012-02-07 13:02:18 patch for preventing the specification of conflicting transaction read/write options
Previous Message Chetan Suttraway 2012-02-07 12:40:29 Re: patch for implementing SPI_gettypemod()