From: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
---|---|
To: | Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Completing PL support for Event Triggers |
Date: | 2013-10-01 03:04:21 |
Message-ID: | 1380596661.22785.9.camel@vanquo.pezone.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Review of the PL/Tcl part: The functionality looks OK. There are some
cosmetic issues. If those are addressed, I think this can be committed.
In the documentation, "Event Triggers" -> "Event triggers".
For the example in the documentation, please show the output, that is,
what the trigger outputs.
Remove the extra space before " tclsnitch".
Document the return value (it is ignored). Will we need the return
value in a future expansion? Should we leave room for that?
Change "command trigger" to "event trigger" in several places.
compile_pltcl_function() does not catch trigger function being called as
event trigger or vice versa. Not sure if that should be covered.
The first PG_TRY block in pltcl_event_trigger_handler() is unnecessary,
because there is nothing in there that can throw an exception.
I'd remove some comments from pltcl_event_trigger_handler(). They were
obviously copied from pltcl_trigger_handler(), but that function is much
longer, so more comments might have been appropriate there.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2013-10-01 03:48:16 | Documentation for SET var_name FROM CURRENT |
Previous Message | Bruce Momjian | 2013-10-01 02:46:20 | Re: record identical operator - Review |