Обсуждение: debug_query_string and multiple statements
While reviewing Joachim Wieland's patch to add a pg_cursors system view, I noticed that the patch assumes that debug_query_string contains the portion of the submitted query string that corresponds to the SQL statement we are currently executing. That is incorrect: debug_query_string contains the *entire* verbatim query string sent by the client. So if the client submits the query string "SELECT 1; SELECT 2;", debug_query_string will contain exactly that string. (psql actually splits queries like the above into two separate FE/BE messages -- to see what I'm referring to, use libpq directly, or start up a copy of the standalone backend.) This makes debug_query_string the wrong thing to use for the pg_cursors and pg_prepared_statements views, but it affects other parts of the system as well: for example, given PQexec(conn, "SELECT 1; SELECT 2/0;") and log_min_error_statement = 'error', the postmaster will log: ERROR: division by zero STATEMENT: SELECT 1; SELECT 2/0; which seems misleading, and is inconsistent with the documentation's description of this configuration parameter. Admittedly this isn't an enormous problem, but I think the current behavior isn't ideal. Unfortunately I don't see an easy way to fix this. It might be possible to extra a semicolon separated list of query strings from the parser or lexer, but that would likely have the effect of munging comments and whitespace from the literal string submitted by the client, which seems the wrong thing to do for logging purposes. An alternative might be to do a preliminary scan to look for semicolon delimited query strings, and then pass each of those strings into the raw_parser() separately, but that seems quite a lot of work (and perhaps a significant runtime cost) to fix what is at worst a minor UI wrinkle. Thoughts? -Neil
Neil Conway <neilc@samurai.com> writes:
> While reviewing Joachim Wieland's patch to add a pg_cursors system view,
> I noticed that the patch assumes that debug_query_string contains the
> portion of the submitted query string that corresponds to the SQL
> statement we are currently executing. That is incorrect:
Yeah, this has annoyed me for some time. debug_query_string is really
just a quick hack and has already been stretched well past its intended
use. I don't think you get the right result for situations where the
active query has been submitted via SPI, either: debug_query_string will
show the outermost interactive command, which may have little to do with
the query that got the error.
The appropriate way to fix it IMHO is to get the lexer+grammar to
identify the statement boundary(s) in the source string and add explicit
support in the parser for saving away the appropriate strings.
This would tie nicely into something else I'd like to do someday, which
is improve parse-analysis error reports by being able to finger the
offending construct more precisely. When we report something like an
unknown variable name in a huge query, it really sucks that we can't
give an error cursor the way simple lexer or grammar errors do. To
fix that, tokens generated by the lexer/grammar need to carry along
text positions ... and the position of the semicolon token is just what
we'd need to create proper statement strings, too.
regards, tom lane
Yep, I couldn't find a better way to do it when I added debug_query_string long ago. Unless we go to a lot of work to parse the string, we could end up with something worse than we have now. --------------------------------------------------------------------------- Neil Conway wrote: > While reviewing Joachim Wieland's patch to add a pg_cursors system view, > I noticed that the patch assumes that debug_query_string contains the > portion of the submitted query string that corresponds to the SQL > statement we are currently executing. That is incorrect: > debug_query_string contains the *entire* verbatim query string sent by > the client. So if the client submits the query string "SELECT 1; SELECT > 2;", debug_query_string will contain exactly that string. (psql actually > splits queries like the above into two separate FE/BE messages -- to see > what I'm referring to, use libpq directly, or start up a copy of the > standalone backend.) > > This makes debug_query_string the wrong thing to use for the pg_cursors > and pg_prepared_statements views, but it affects other parts of the > system as well: for example, given PQexec(conn, "SELECT 1; SELECT 2/0;") > and log_min_error_statement = 'error', the postmaster will log: > > ERROR: division by zero > STATEMENT: SELECT 1; SELECT 2/0; > > which seems misleading, and is inconsistent with the documentation's > description of this configuration parameter. Admittedly this isn't an > enormous problem, but I think the current behavior isn't ideal. > > Unfortunately I don't see an easy way to fix this. It might be possible > to extra a semicolon separated list of query strings from the parser or > lexer, but that would likely have the effect of munging comments and > whitespace from the literal string submitted by the client, which seems > the wrong thing to do for logging purposes. An alternative might be to > do a preliminary scan to look for semicolon delimited query strings, and > then pass each of those strings into the raw_parser() separately, but > that seems quite a lot of work (and perhaps a significant runtime cost) > to fix what is at worst a minor UI wrinkle. > > Thoughts? > > -Neil > > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
If we want to save the SQL statement for some database objects(table, view, etc.), the backend will see the same problem. Here is an example. create table s(sno int, sname char(10)); select 1; I recall that some DBMS will store the statement for table s like this: create table s(sno int, sname char(10)); We should also treat the comments. "Neil Conway" <neilc@samurai.com> wrote message > While reviewing Joachim Wieland's patch to add a pg_cursors system view, > I noticed that the patch assumes that debug_query_string contains the > portion of the submitted query string that corresponds to the SQL > statement we are currently executing. That is incorrect: > debug_query_string contains the *entire* verbatim query string sent by > the client. So if the client submits the query string "SELECT 1; SELECT > 2;", debug_query_string will contain exactly that string. (psql actually > splits queries like the above into two separate FE/BE messages -- to see > what I'm referring to, use libpq directly, or start up a copy of the > standalone backend.) > > This makes debug_query_string the wrong thing to use for the pg_cursors > and pg_prepared_statements views, but it affects other parts of the > system as well: for example, given PQexec(conn, "SELECT 1; SELECT 2/0;") > and log_min_error_statement = 'error', the postmaster will log: > > ERROR: division by zero > STATEMENT: SELECT 1; SELECT 2/0; > > which seems misleading, and is inconsistent with the documentation's > description of this configuration parameter. Admittedly this isn't an > enormous problem, but I think the current behavior isn't ideal. > > Unfortunately I don't see an easy way to fix this. It might be possible > to extra a semicolon separated list of query strings from the parser or > lexer, but that would likely have the effect of munging comments and > whitespace from the literal string submitted by the client, which seems > the wrong thing to do for logging purposes. An alternative might be to > do a preliminary scan to look for semicolon delimited query strings, and > then pass each of those strings into the raw_parser() separately, but > that seems quite a lot of work (and perhaps a significant runtime cost) > to fix what is at worst a minor UI wrinkle. > > Thoughts? > > -Neil > > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend >