Re: Patch review for logging hooks (CF 2012-01) - Mailing list pgsql-hackers
From | Andrew Dunstan |
---|---|
Subject | Re: Patch review for logging hooks (CF 2012-01) |
Date | |
Msg-id | 4F54FCDD.2050709@dunslane.net Whole thread Raw |
In response to | Re: Patch review for logging hooks (CF 2012-01) (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Patch review for logging hooks (CF 2012-01)
(Robert Haas <robertmhaas@gmail.com>)
|
List | pgsql-hackers |
On 03/05/2012 12:08 PM, Tom Lane wrote: > Robert Haas<robertmhaas@gmail.com> writes: >> On Sun, Mar 4, 2012 at 10:45 AM, Andrew Dunstan<andrew@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. src/test/api >>> or something similar? I'm not sure how we'd automate a test for this case, >>> though. I guess we could use something like pg_logforward and have a UDP >>> receiver catch the messages and write them to a file. Something like that >>> should be possible to rig up in Perl. But all that seems a lot of work at >>> this stage of the game. So the question is do we want to commit this patch >>> without it? >> 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 generally asked for with hooks is a working sample usage of > the hook, just as a cross-check that something useful can be done with > it and you didn't overlook any obvious usability problems. I agree that > a regression test is often not practical, especially not if you're not > prepared to create a whole contrib module to provide a sample usage. > > In the case at hand, ISTM there are some usability questions around > where/when the hook is called: in particular, if I'm reading it right, > the hook could not override a log_min_messages-based decision that a > given message is not to be emitted. Do we care? That's what I understand too. We could relax that at some stage in the future if we had a requirement, I guess. > Also, if the hook > is meant to be able to change the data that gets logged, as seems to be > the case, do we care that it would also affect what gets sent to the > client? > > 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 only be able to change output_to_server and output_to_client, and possibly only to change them from true to false (i.e. I'm not sure the hook should be able to induce more verbose logging.) But maybe that's too restrictive. I doubt we can enforce good behaviour, though, only state that if you break things you get to keep all the pieces. cheers andrew
pgsql-hackers by date: