On Tue, Mar 6, 2012 at 10:37 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Applied with minor editorialization (mainly just improving the
comments).
Thanks and kudos to all the reviewers!
regards,
Martin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your
Martin Pihlak martin.pih...@gmail.com writes:
Updated patch attached.
Applied with minor editorialization (mainly just improving the
comments).
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
On Sun, Mar 4, 2012 at 10:45 AM, Andrew Dunstan and...@dunslane.net wrote:
I'm just looking at this patch, and I agree, it should be testable. I'm
wondering if it wouldn't be a good idea to have a module or set of modules
for demonstrating and testing bits of the API that we expose.
Robert Haas robertmh...@gmail.com writes:
On Sun, Mar 4, 2012 at 10:45 AM, Andrew Dunstan and...@dunslane.net wrote:
I'm just looking at this patch, and I agree, it should be testable. I'm
wondering if it wouldn't be a good idea to have a module or set of modules
for demonstrating and testing
On 03/05/2012 12:08 PM, Tom Lane wrote:
Robert Haasrobertmh...@gmail.com writes:
On Sun, Mar 4, 2012 at 10:45 AM, Andrew Dunstanand...@dunslane.net wrote:
I'm just looking at this patch, and I agree, it should be testable. I'm
wondering if it wouldn't be a good idea to have a module or set
On Mon, Mar 5, 2012 at 12:50 PM, Andrew Dunstan and...@dunslane.net wrote:
The latest version of this patch looks sound to me. We haven't
insisted on having even a sample application for every hook before,
let alone a regression test, so I don't think this patch needs one
either.
What we've
Robert Haas robertmh...@gmail.com writes:
On Mon, Mar 5, 2012 at 12:50 PM, Andrew Dunstan and...@dunslane.net wrote:
I'd like to see a spec for exactly which fields of ErrorData the hook is
allowed to change, and some rationale.
Good question. I'd somewhat be inclined to say that it should
On 01/20/2012 10:01 AM, Robert Haas wrote:
On Fri, Jan 20, 2012 at 2:47 AM, Greg Smithg...@2ndquadrant.com wrote:
The updated patch looks good, marking as 'Ready for Committer'
Patches without documentation are never ready for commit. For this one, I'm
not sure if that should be in the
On Fri, Jan 20, 2012 at 2:47 AM, Greg Smith g...@2ndquadrant.com wrote:
The updated patch looks good, marking as 'Ready for Committer'
Patches without documentation are never ready for commit. For this one, I'm
not sure if that should be in the form of a reference example in contrib, or
just
On Fri, Jan 20, 2012 at 17:01, Robert Haas robertmh...@gmail.com wrote:
We should probably at least have a
working example for testing purposes, though, whether or not we end up
committing it.
Martin Pihlak sent a short description of how to test the patch with
his pg_logforward module:
On 01/18/2012 04:23 PM, Marti Raudsepp wrote:
The updated patch looks good, marking as 'Ready for Committer'
Patches without documentation are never ready for commit. For this one,
I'm not sure if that should be in the form of a reference example in
contrib, or just something that documents
On 01/18/2012 03:56 AM, Fujii Masao wrote:
or syslog process (if you use syslog). So ISTM that there is no
guarantee that the order of log messages processed by the
hook is same as that of messages written to the log file. For
example, imagine the case where two backends call EmitErrorReport()
On 01/18/2012 11:12 AM, Martin Pihlak wrote:
On 01/18/2012 03:56 AM, Fujii Masao wrote:
or syslog process (if you use syslog). So ISTM that there is no
guarantee that the order of log messages processed by the
hook is same as that of messages written to the log file. For
example, imagine the
Excerpts from Andrew Dunstan's message of mié ene 18 13:27:40 -0300 2012:
On 01/18/2012 11:12 AM, Martin Pihlak wrote:
On 01/18/2012 03:56 AM, Fujii Masao wrote:
or syslog process (if you use syslog). So ISTM that there is no
guarantee that the order of log messages processed by the
On 01/17/2012 11:40 PM, Marti Raudsepp wrote:
It seems you missed a comment, that the current implementation is also
affected by client_min_messages. I think that being affected by
client-specific settings is surprising. I would put the
if(emit_log_hook) inside the existing
On Wed, Jan 18, 2012 at 22:56, Martin Pihlak martin.pih...@gmail.com wrote:
We need to keep the if separate
though -- the hook might want to omit the message from server
log so the output_to_server needs to be rechecked.
Oh, yes makes sense.
The updated patch looks good, marking as 'Ready for
Hi,
Here's my review for the logging hooks patch queued for the 2012-01
CommitFest by Martin Pihlak.
Submission review
The patch is in context diff format and applies fine. Tests are not
included and don't seem practical for this patch.
More documentation would always be nice, but as other
On 01/17/2012 07:35 PM, Marti Raudsepp wrote:
Here's my review for the logging hooks patch queued for the 2012-01
CommitFest by Martin Pihlak.
Thanks for reviewing!
There's a minor whitespace problem. When declaring variables, and the
data type is longer than 12 characters, just use 1
Hi!
On Tue, Jan 17, 2012 at 23:07, Martin Pihlak martin.pih...@gmail.com wrote:
I think the hook warrants a comment that, whether the messages will be
seen, depends on the log_min_messages setting.
Comment added.
Nice :)
It seems you missed a comment, that the current implementation is also
On Wed, Jan 18, 2012 at 2:35 AM, Marti Raudsepp ma...@juffo.org wrote:
Are there corner cases the author has failed to consider?
The hook can be executed by various processes since it's in
EmitErrorReport(). OTOH, log messages are written to the log
file by one process like syslogger (if you use
20 matches
Mail list logo
