On 24 February 2012 22:39, Thom Brown <t...@linux.com> wrote: > On 24 February 2012 22:32, Thom Brown <t...@linux.com> wrote: >> On 24 February 2012 22:04, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote: >>> Hi, >>> >>> Please find attached the latest version of the command triggers patch, >>> in context diff format, with support for 79 commands and documentation >>> about why only those, and with some limitations explained. >>> >>> I also cleaned up the node function support business that was still in >>> there from the command rewriting stuff that we dropped, and did a merge >>> from tonight's master branch (one of a few clean merges). >>> >>> This is now the whole of it, and I continue being available to make any >>> necessary change, although I expect only minor changes now. Thanks to >>> all reviewers and participants into the previous threads, you all have >>> allowed me to reach the current point by your precious advice, comments >>> and interest. >>> >>> The patch implements : >>> >>> - BEFORE/AFTER ANY command triggers >>> - BEFORE/AFTER command triggers for 79 documented commands >>> - regression tests exercised by the serial schedule only >>> - documentation updates with examples >>> >>> That means you need to `make installcheck` here. Installing the tests in >>> the parallel schedule does not lead to consistent output as installing a >>> command trigger will impact any other test using that command, and the >>> output becomes subject to the exact ordering of the concurrent tests. >>> >>> The only way for a BEFORE command triggers to change the command's >>> behaviour is by raising an exception that aborts the whole transaction. >>> >>> Command triggers are called with the following arguments: >>> >>> - the “event” (similar to TG_WHEN, either 'BEFORE' or 'AFTER') >>> - the command tag (the real one even when an ANY trigger is called) >>> - the object Id if available (e.g. NULL for a CREATE statement) >>> - the schema name (can be NULL) >>> - the object name (can be NULL) >>> >>> When the trigger's procedure we're calling is written in C, then another >>> argument is passed next, which is the parse tree Node * pointer. >>> >>> I've been talking with Marko Kreen about supporting ALTER TABLE and such >>> commands automatically in Londiste: given that patch, it requires >>> writing code in C that will rewrite the command string. It so happens >>> that I already have worked on that code, so we intend on bringing >>> support for ALTER TABLE and other commands in Skytools 3 for 9.2. >>> >>> I think the support code should be made into an extension that both >>> Skytools and Slony would be able to share. The extension code will be >>> able to adapt to major versions changes as they are released. Bucardo >>> would certainly be interested too, we could NOTIFY it from such an >>> extension. The design is yet to be done here, but it's clearly possible >>> to implement such a feature given the current patch. >>> >>> The ANY trigger support is mainly there to allow people to stop any DDL >>> traffic on their databases, then allowing it explicitly with an ALTER >>> COMMAND TRIGGER ... SET DISABLE when appropriate only. To that >>> end, the ANY command trigger is supporting more commands than you can >>> attach specific triggers too. >>> >>> It's also possible to ENABLE a command trigger on the REPLICA only >>> thanks to the session_replication_role GUC. Support for command >>> triggers on an Hot Standby node is not provided in this patch. >> >> Hi Dimitri, >> >> I just tried building the docs with your patch and it appears >> doc/src/sgml/ref/allfiles.sgml hasn't been updated with the necessary >> references for alterCommandTrigger, createCommandTrigger and >> dropCommandTrigger. >> >> Also in ref/alter_command_trigger.sgml, you define SQL-CREATETRIGGER. >> Shouldn't this be SQL-CREATECOMMANDTRIGGER? And there also appears to >> be orphaned text in the file too, such as "Forbids the execution of >> any DDL command". And there's a stray </para> on line 299. >> >> I attach updated versions of both of those files, which seems to fix >> all these problems. > > I've just noticed there's an issue with > doc/src/sgml/ref/alter_command_trigger.sgml too. It uses <indexterm > zone="sql-altertrigger"> which should be sql-altercommandtrigger. (as > attached)
And upon trying to test the actual feature, it didn't work for me at all. I thought I had applied the patch incorrectly, but I hadn't, it was the documentation showing the wrong information. The CREATE COMMAND TRIGGER page actually just says CREATE TRIGGER.... BEFORE COMMAND <command>, which isn't the correct syntax. Also the examples on the page are incorrect in the same regard. When I tested it with the correction, I got an error saying that the function used had to return void, but the example uses bool. Upon also changing this, the example works as expected. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
