On Tue, Mar 20, 2012 at 1:49 PM, Dimitri Fontaine
<dimi...@2ndquadrant.fr> wrote:
> Hi,
> I guess I sent v17 a little early considering that we now already have
> v18 including support for CREATE TABLE AS and SELECT INTO, thanks to the
> work of Andres and Tom.
> There was some spurious tags in the sgml files in v17 that I did clean
> up too.

There are spurious whitespace changes in the documentation.  Some of
these are of the following form:

- return_arr
+ return_arr

create_trigger.sgml adds a stray blank line as well.

I think that the syntax for enabling or disabling a command trigger
should not use the keyword SET.  So just:


That way is more parallel with the existing syntax for ordinary triggers.

+      The name to give the new trigger. This must be distinct from the name
+      of any other trigger for the same table. The name cannot be
+      schema-qualified.

"For the same table" is a copy-and-pasteo.

-               /* Look up object address. */
+               /* Look up object address. */

Spurious diff.

I think that get_object_address_cmdtrigger should be turned into a
case within get_object_address_unqualified; I can't see a reason for
it to have its own function.

+               elog(ERROR, "could not find tuple for command trigger
%u", trigOid);

The standard phrasing is "cache lookup failed for command trigger %u".

+ * Functions to execute the command triggers.
+ *
+ * We call the functions that matches the command triggers definitions in
+ * alphabetical order, and give them those arguments:
+ *
+ *   command tag, text
+ *   objectId, oid
+ *   schemaname, text
+ *   objectname, text
+ *
+ */

This will not survive pgindent, unless you surround it with ------ and
-----.  More generally, it would be nice if you could pgindent the
whole patch and then fix anything that breaks in such a way that it
will survive the next pgindent run.

+ * if (CommandFiresTriggers(cmd)

Missing paren.

+       /* before or instead of command trigger might have cancelled
the command */


@@ -169,7 +169,7 @@ ExplainQuery(ExplainStmt *stmt, const char *queryString,
                                 errmsg("EXPLAIN option BUFFERS
requires ANALYZE")));
        /* if the timing was not set explicitly, set default value */
        es.timing = (timing_set) ? es.timing : es.analyze;

Spurious diff.

All the changes to ruleutils.c appear spurious.  Ditto the change to

In the pg_dump support, it seems you're going to try to execute an
empty query if this is run against a pre-9.2 server.  It seems like
you should just return, or something like that.  What do we do in
comparable cases elsewhere?

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to