Обсуждение: selective statement logging
The TODO list contains this item which I said I would look at:
Allow logging of only data definition(DDL), or DDL and modification
statements
The trouble I see is that we currently do statement logging before we
have examined the query string at all, in the code shown below from
src/backend/tcop/postgres.c.
I guess I could construct one or more regexes to examine the query
string, although that might affect performance a bit (of course, I would
precompile the patterns).
Any other ideas on how to proceed?
cheers
andrew
List *
pg_parse_query(const char *query_string)
{ List *raw_parsetree_list;
if (log_statement) ereport(LOG, (errmsg("statement: %s", query_string)));
if (log_parser_stats) ResetUsage();
raw_parsetree_list = raw_parser(query_string);
if (log_parser_stats) ShowUsage("PARSER STATISTICS");
return raw_parsetree_list;
}
Andrew Dunstan wrote: > > The TODO list contains this item which I said I would look at: > > Allow logging of only data definition(DDL), or DDL and modification > statements > > The trouble I see is that we currently do statement logging before we > have examined the query string at all, in the code shown below from > src/backend/tcop/postgres.c. > > I guess I could construct one or more regexes to examine the query > string, although that might affect performance a bit (of course, I would > precompile the patterns). > > Any other ideas on how to proceed? Yes, look at how the command tag is grabbed for the PS display, and do the log checks at that point. -- 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
Bruce Momjian wrote: >Andrew Dunstan wrote: > > >>The TODO list contains this item which I said I would look at: >> >> Allow logging of only data definition(DDL), or DDL and modification >>statements >> >>The trouble I see is that we currently do statement logging before we >>have examined the query string at all, in the code shown below from >>src/backend/tcop/postgres.c. >> >>I guess I could construct one or more regexes to examine the query >>string, although that might affect performance a bit (of course, I would >>precompile the patterns). >> >>Any other ideas on how to proceed? >> >> > >Yes, look at how the command tag is grabbed for the PS display, and do >the log checks at that point. > > > Yes, I thought about that. But it would delay the logging of statements, and I'm not sure that's a good idea. What would happen on parse errors, for example? cheers andrew
Andrew Dunstan wrote: > >Yes, look at how the command tag is grabbed for the PS display, and do > >the log checks at that point. > > > > > > > Yes, I thought about that. But it would delay the logging of statements, > and I'm not sure that's a good idea. What would happen on parse errors, > for example? Well, if it is parse error, then we can't know what type of command it really was. They could type 'SE9ECT 1' or 'SELECT 1 WH8RE x=1' and both are not really SELECT commands to me. -- 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
Bruce Momjian wrote: >Andrew Dunstan wrote: > > >>>Yes, look at how the command tag is grabbed for the PS display, and do >>>the log checks at that point. >>> >>> >>> >>> >>> >>Yes, I thought about that. But it would delay the logging of statements, >>and I'm not sure that's a good idea. What would happen on parse errors, >>for example? >> >> > >Well, if it is parse error, then we can't know what type of command it >really was. They could type 'SE9ECT 1' or 'SELECT 1 WH8RE x=1' and both >are not really SELECT commands to me. > > > If I delay logging until after the parse, these log lines would come out in reverse order. Not sure if that matters to anyone, but it might annoy me slightly. line:3 LOG: statement: se4dt ddd; line:4 ERROR: syntax error at or near "se4dt" at character 1 cheers andrew
Andrew Dunstan wrote: > >Well, if it is parse error, then we can't know what type of command it > >really was. They could type 'SE9ECT 1' or 'SELECT 1 WH8RE x=1' and both > >are not really SELECT commands to me. > > > > > > > > If I delay logging until after the parse, these log lines would come out > in reverse order. Not sure if that matters to anyone, but it might annoy > me slightly. > > line:3 LOG: statement: se4dt ddd; > line:4 ERROR: syntax error at or near "se4dt" at character 1 Sure you sure? I didn't think you would get a tag on a syntax error, so no log would print, which I think is OK. -- 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
Bruce Momjian wrote: >Andrew Dunstan wrote: > > >>>Well, if it is parse error, then we can't know what type of command it >>>really was. They could type 'SE9ECT 1' or 'SELECT 1 WH8RE x=1' and both >>>are not really SELECT commands to me. >>> >>> >>> >>> >>> >>If I delay logging until after the parse, these log lines would come out >>in reverse order. Not sure if that matters to anyone, but it might annoy >>me slightly. >> >>line:3 LOG: statement: se4dt ddd; >>line:4 ERROR: syntax error at or near "se4dt" at character 1 >> >> > >Sure you sure? I didn't think you would get a tag on a syntax error, so >no log would print, which I think is OK. > > > If people are happy with suppressing statement logging on a parse error, OK. For the remainder I would just defer the logging till immediately after the parse and get the tags for the statements - and suppress logging the query string if they were all SELECT or all SELECT|INSERT|UPDATE|DELETE|COPY according to the setting. cheers andrew
Andrew Dunstan wrote: > >Sure you sure? I didn't think you would get a tag on a syntax error, so > >no log would print, which I think is OK. > > > > > > > If people are happy with suppressing statement logging on a parse error, > OK. For the remainder I would just defer the logging till immediately > after the parse and get the tags for the statements - and suppress > logging the query string if they were all SELECT or all > SELECT|INSERT|UPDATE|DELETE|COPY according to the setting. Good. My feeling is that if there is a parse error, we can't be sure what type of query was sent, and it isn't going to be executed anyway because it threw an error. -- 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
Andrew Dunstan <andrew@dunslane.net> writes:
> If people are happy with suppressing statement logging on a parse error,
> OK.
I think that's a really, really awful idea. Not infrequently, the
postmaster log is the easiest way of debugging applications that are
sending bogus SQL. If you fail to log the bogus SQL then you've just
cut debuggers off at the knees.
I haven't read the earlier part of the thread yet so I don't know just
what problem you want to solve, but please not this solution.
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > If people are happy with suppressing statement logging on a parse error, > > OK. > > I think that's a really, really awful idea. Not infrequently, the > postmaster log is the easiest way of debugging applications that are > sending bogus SQL. If you fail to log the bogus SQL then you've just > cut debuggers off at the knees. > > I haven't read the earlier part of the thread yet so I don't know just > what problem you want to solve, but please not this solution. The issue is allowing only logging of DDL statements, or DDL and data modification statements, as listed on the TODO list. If they ask for all statements, certainly we should log all statements. -- 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
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>If people are happy with suppressing statement logging on a parse error, >>OK. >> >> > >I think that's a really, really awful idea. Not infrequently, the >postmaster log is the easiest way of debugging applications that are >sending bogus SQL. If you fail to log the bogus SQL then you've just >cut debuggers off at the knees. > >I haven't read the earlier part of the thread yet so I don't know just >what problem you want to solve, but please not this solution. > > > I had a small bet with myself that you'd say that :-) I agree with you. Actually, I think I can improve the present situation. Currently, if log_statement is not turned on and you send a query that doesn't parse, all you get is the error trace. By deferring it till right after the parse we can force logging of the query string on a parse error, regardless of that setting (which seems to me to be a very desirable outcome). The only thing is that you will get the error trace first (because it comes from the parser) rather than the query string first. That should keep you happy, I hope ;-) (The problem being addressed in this thread is to allow selective logging of DDL/DML statements - see the TODO list. Someone was actually asking for exactly this on irc today.). cheers andrew
Andrew Dunstan wrote: > > Actually, I think I can improve the present situation. Currently, if > log_statement is not turned on and you send a query that doesn't > parse, all you get is the error trace. By deferring it till right > after the parse we can force logging of the query string on a parse > error, regardless of that setting (which seems to me to be a very > desirable outcome). The only thing is that you will get the error > trace first (because it comes from the parser) rather than the query > string first. > Or I would do except that I see that a parse error causes a longjmp out of that code. I will see if I can come up with a way around that. If not, I think we'd better let sleeping dogs lie. cheers andrew
Bruce Momjian <pgman@candle.pha.pa.us> writes: > The issue is allowing only logging of DDL statements, or DDL and data > modification statements, as listed on the TODO list. If they ask for > all statements, certainly we should log all statements. just make syntax errors one of the types. So you could log "ddl", "dml", "syntax errors", or "all" statements. In fact for production use syntax errors seem like the most likely thing to want to log. -- greg