From: Dimitri Fontaine <dimitri@2ndQuadrant.fr>
To: Thom Brown <thom@linux.com>
Cc: Dimitri Fontaine <dimitri@2ndquadrant.fr>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Command Triggers, patch v11
Date: Sat, 25 Feb 2012 13:00:53 +0100
Message-ID: <m2y5rrktqi.fsf@2ndQuadrant.fr> (raw)
In-Reply-To: <CAA-aLv5H5_ALMRZFatX-wDEz1meMvQVJOsBBFTYox65jzAoN5Q@mail.gmail.com>
References: <m24nufq466.fsf@2ndQuadrant.fr>
<CAA-aLv5H5_ALMRZFatX-wDEz1meMvQVJOsBBFTYox65jzAoN5Q@mail.gmail.com>
Hi,
Please find attached version 12 of the patch, which is fixing docs per
your review. Thanks for your time, comments and fixes!
You can see the patch-on-patch here for quick proof reading:
https://github.com/dimitri/postgres/commit/b7798e8ba6c9bee1f65b233316ae9c08b78e5ddb
Thom Brown <thom@linux.com> writes:
> 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.
Those are in the attached, apart from your editing of the examples para.
A single para is needed around all examples, which was forgotten in my
previous version of the patch, now fixed.
Thom Brown <thom@linux.com> writes:
> 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)
Included.
Thom Brown <thom@linux.com> writes:
> 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.
Seems like I've forgotten to update the docs when acting on Robert's
suggestion to improve the syntax to CREATE COMMAND TRIGGER. I've now
fixed that.
> 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.
Fixed too.
Thom Brown <thom@linux.com> writes:
> Is there any reason why the list of commands that command triggers can
> be used with isn't in alphabetical order? Also it appears to show
Any reason why? I don't suppose it's really important one way or the
other, so I'm waiting on some more voices before working on it.
> CREATE/ALTER/DROP TYPE_P, and the same for CONVERSION_P and DOMAIN_P.
> I'm assuming these are typos? They also appear on DROP COMMAND
> TRIGGER.
Yeah I did use an emacs macro to get from the gram.y format to the docs
format, then replaced '_P ' with ''. Should have replaced '_P' really,
now done.
> The ALTER COMMAND TRIGGER page also doesn't show which commands it can
> be used against. Perhaps, rather than repeat the list, there could be
> a note to say that a list of valid commands can be found on the CREATE
> COMMAND TRIGGER page?
Well you can only alter a command that you were successful in creating,
right? So I'm not sure that's needed here. By that count though, I
maybe should remove the supported command list from DROP COMMAND TRIGGER
reference page?
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
command-trigger.v12.patch.gz (application/octet-stream, 58.5K) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: dimitri@2ndQuadrant.fr, thom@linux.com
Subject: Re: Command Triggers, patch v11
In-Reply-To: <m2y5rrktqi.fsf@2ndQuadrant.fr>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox