On 28 Mar 2008, at 17:23, Bruce Momjian wrote:
Neil Conway wrote:
On Mon, 2007-07-05 at 19:48 +0100, Tomas Doran wrote:
As suggested in the TODO list (and as I need the functionality
myself), I have implemented the current_query interface to
debug_query_string.

It actually has been removed from the TODO list since you saw it last.

I submitted a patch to make it do that a while ago :)

Comments:

...
* AFAIK debug_query_string() still does the wrong thing when the user
submits multiple queries in a single protocol message (separated by
semi-colons). Not sure there's a way to fix that that is both easy and
efficient, though...

The problem with the last bullet is pretty serious.  It can be
illustrated with psql:

        $ psql -c 'set log_statement="all";select 1;select 2;' test

Server log shows:

        STATEMENT:  set log_statement=all;select 1;select 2;

Obviously this is what current_query() would return if we commit this
patch, and it probably isn't 100% accurate.

Yeah, this was pointed out to me at the time.

Fortunately, for what I wanted to do, 'Don't do that then' was a very viable answer..

I see dblink exposes this:

http://www.postgresql.org/docs/8.3/static/contrib-dblink- current-query.html

    Returns the currently executing interactive command string of the
    local database session, or NULL if it can't be determined.  Note
    that this function is not really related to <filename>dblink</>'s
    other functionality.  It is provided since it is sometimes useful
    in generating queries to be forwarded to remote databases.

My patch provided this functionality in core, and made dblink's current procedure to do the same just delegate to the one that I provided (for backwards compatibility reasons)


but making it more widely available with a possible inaccurate result is
a problem.  We can't think of anyway to fix this cleanly --- it would
require a separate parser pass to split queries by semicolons (which
psql does by default in interactive mode).  Right now the parser does
the splitting as part of its normal single-parse operation and just
creates parse trees that don't have string representations.

Perhaps we could name it received_query() to indicate it is what the
backend received and it not necessarily the _current_ query.

reveived_query() sounds like a very sane name for me, and documenting it as such would allow you to expose the functionality without the possible complaints...

In a lot of environments where you actually want this, then constraining to 1 query per statement (outside the DB level) is very doable... I wouldn't like to see the functionality skipped over as providing this only solves 80% of cases.


In the particular application that I wrote the patch for, we needed to audit 'all access to encrypted credit card numbers' for PCI requirements..

Our solution was to put all cc number containing tables into their own schema / with no general permissions, and to use SECURITY DEFINER stored procedures to access them (and log the access).. However that wasn't quite good enough, so we got our DB access layer to iterate up the call stack (till outside our SQL abstraction), and add a comment to every query such that it took the form:

/* CodeFile-LineNo-UserId */ SELECT stored_procedure(arg1, arg2);

for all queries - so the caller information was encoded in the query info... Therefore, inside 'stored_procedure', logging the value of current_query() was perfect to satisfy our audit requirements, and we can just log the current query when we enter 'stored_procedure'.

Hope this helps to clarify that, whilst the current mechanism isn't in any way perfect - there are a number of use cases for including the functionality as-is.

Cheers
Tom


Cheers
Tom


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

Reply via email to