On 03/05/2012 12:08 PM, Tom Lane wrote:
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 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
