Re: [PATCHES] [HACKERS] logging statement levels
Andrew Dunstan wrote: Here are some options: 1. change the type of log_statement option from boolean to string, with allowed values of all, mod, ddl, none with default none. 2. same as 1. but make boolean true values synonyms for all and boolean false values synonyms for none. 3. keep log_statement option as now and add a new option log_statement_level with the same options as 1. but default to all, which will have no effect unless log_statement is true. I like 1. OK, here is a patch that implements #1. Here is sample output: test= set client_min_messages = 'log'; SET test= set log_statement = 'mod'; SET test= select 1; ?column? -- 1 (1 row) test= update test set x=1; LOG: statement: update test set x=1; ERROR: relation test does not exist test= update test set x=1; LOG: statement: update test set x=1; ERROR: relation test does not exist test= copy test from '/tmp/x'; LOG: statement: copy test from '/tmp/x'; ERROR: relation test does not exist test= copy test to '/tmp/x'; ERROR: relation test does not exist test= prepare xx as select 1; PREPARE test= prepare xx as update x set y=1; LOG: statement: prepare xx as update x set y=1; ERROR: relation x does not exist test= explain analyze select 1;; QUERY PLAN Result (cost=0.00..0.01 rows=1 width=0) (actual time=0.006..0.007 rows=1 loops=1) Total runtime: 0.046 ms (2 rows) test= explain analyze update test set x=1; LOG: statement: explain analyze update test set x=1; ERROR: relation test does not exist test= explain update test set x=1; ERROR: relation test does not exist It checks PREPARE and EXECUTE ANALYZE too. The log_statement values are 'none', 'mod', 'ddl', and 'all'. For 'all', it prints before the query is parsed, and for ddl/mod, it does it right after parsing using the node tag (or command tag for CREATE/ALTER/DROP), so any non-parse errors will print after the log line. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/runtime.sgml === RCS file: /cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v retrieving revision 1.257 diff -c -c -r1.257 runtime.sgml *** doc/src/sgml/runtime.sgml 5 Apr 2004 03:02:03 - 1.257 --- doc/src/sgml/runtime.sgml 6 Apr 2004 03:56:08 - *** *** 2121,2132 /varlistentry varlistentry id=guc-log-statement xreflabel=log_statement ! termvarnamelog_statement/varname (typeboolean/type)/term listitem para ! Causes each SQL statement to be logged. The default is ! off. Only superusers can disable this option if it has been ! enabled by an administrator. /para note --- 2121,2141 /varlistentry varlistentry id=guc-log-statement xreflabel=log_statement ! termvarnamelog_statement/varname (typestring/type)/term listitem para ! Controls which SQL statement are logged. Valid values are ! literalall/, literalddl/, literalmod/, and ! literalnone/. literalddl/ logs all data definition ! commands like literalCREATE/, literalALTER/, and ! literalDROP/ commands. literalmod/ logs all ! literalddl/ statements, plus literalINSERT/, ! literalUPDATE/, literalDELETE/, literalTRUNCATE/, ! and literalCOPY FROM/. literalPREPARE/ and ! literalEXPLAIN ANALYZE/ statements are also considered for ! appropriate commands. The default is literalnone/. Only ! superusers can reduce the detail of this option if it has been ! set by an administrator. /para note Index: src/backend/tcop/postgres.c === RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v retrieving revision 1.397 diff -c -c -r1.397 postgres.c *** src/backend/tcop/postgres.c 24 Mar 2004 22:40:29 - 1.397 --- src/backend/tcop/postgres.c 6 Apr 2004 03:56:11 - *** *** 87,92 --- 87,94 /* flag for logging end of session */ boolLog_disconnections = false; + LogStmtLevel log_statement = LOGSTMT_NONE; + /* * Flags for expensive function optimization -- JMH 3/9/92 */ *** *** 471,479 List * pg_parse_query(const
Re: [PATCHES] [HACKERS] logging statement levels
Unless I'm missing something, this patch has the effect that with values of ddl or mod for log_statement, a statement with a parse error will not be logged, which was what I hoped to avoid. cheers andrew Bruce Momjian wrote: Andrew Dunstan wrote: Here are some options: 1. change the type of log_statement option from boolean to string, with allowed values of all, mod, ddl, none with default none. 2. same as 1. but make boolean true values synonyms for all and boolean false values synonyms for none. 3. keep log_statement option as now and add a new option log_statement_level with the same options as 1. but default to all, which will have no effect unless log_statement is true. I like 1. OK, here is a patch that implements #1. Here is sample output: test= set client_min_messages = 'log'; SET test= set log_statement = 'mod'; SET test= select 1; ?column? -- 1 (1 row) test= update test set x=1; LOG: statement: update test set x=1; ERROR: relation test does not exist test= update test set x=1; LOG: statement: update test set x=1; ERROR: relation test does not exist test= copy test from '/tmp/x'; LOG: statement: copy test from '/tmp/x'; ERROR: relation test does not exist test= copy test to '/tmp/x'; ERROR: relation test does not exist test= prepare xx as select 1; PREPARE test= prepare xx as update x set y=1; LOG: statement: prepare xx as update x set y=1; ERROR: relation x does not exist test= explain analyze select 1;; QUERY PLAN Result (cost=0.00..0.01 rows=1 width=0) (actual time=0.006..0.007 rows=1 loops=1) Total runtime: 0.046 ms (2 rows) test= explain analyze update test set x=1; LOG: statement: explain analyze update test set x=1; ERROR: relation test does not exist test= explain update test set x=1; ERROR: relation test does not exist It checks PREPARE and EXECUTE ANALYZE too. The log_statement values are 'none', 'mod', 'ddl', and 'all'. For 'all', it prints before the query is parsed, and for ddl/mod, it does it right after parsing using the node tag (or command tag for CREATE/ALTER/DROP), so any non-parse errors will print after the log line. ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] logging statement levels
Andrew Dunstan wrote: Unless I'm missing something, this patch has the effect that with values of ddl or mod for log_statement, a statement with a parse error will not be logged, which was what I hoped to avoid. Right. The query type can't be determined during a syntax error because the parser couldn't identify the supplied command. I think that is fine. What it does allow is to for 'all' to display the command before the syntax error. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [HACKERS] logging statement levels
Bruce Momjian wrote: Andrew Dunstan wrote: Unless I'm missing something, this patch has the effect that with values of ddl or mod for log_statement, a statement with a parse error will not be logged, which was what I hoped to avoid. Right. The query type can't be determined during a syntax error because the parser couldn't identify the supplied command. I think that is fine. What it does allow is to for 'all' to display the command before the syntax error. If I had to make a choice I'd go the other way. However, I think with a little extra work it might be possible to have both. I will look into it at some stage. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] logging statement levels
Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: Andrew Dunstan wrote: Unless I'm missing something, this patch has the effect that with values of ddl or mod for log_statement, a statement with a parse error will not be logged, which was what I hoped to avoid. Right. The query type can't be determined during a syntax error because the parser couldn't identify the supplied command. I think that is fine. What it does allow is to for 'all' to display the command before the syntax error. If I had to make a choice I'd go the other way. Uh, what other way? reverse the order rather than suppress the message. However, I think with a little extra work it might be possible to have both. Right now, the way it is done, only a real syntax error skips logging. If you referenced an invalid table or something, it does print the log just before the invalid table name mention. How would we test the command type before hitting a syntax error? I can't think of a way, and I am not sure it would even be meaningful. I agree that you can't test the statement type on a parse error. But that doesn't mean to me that mod should suppress logging statements with syntax errors. In fact, after the discussion surrounding this I thought the consensus was to have these things as additive rather than just one level selected. How to do it in the order you prefer? I would trap the parse error and log the statement before emitting the error log. If I find a simple way I'll submit a further patch. Certainly your patch contains the guts of what needs to be done in any case. cheers andrew cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] logging statement levels
Andrew Dunstan wrote: Bruce Momjian wrote: Andrew Dunstan wrote: Unless I'm missing something, this patch has the effect that with values of ddl or mod for log_statement, a statement with a parse error will not be logged, which was what I hoped to avoid. Right. The query type can't be determined during a syntax error because the parser couldn't identify the supplied command. I think that is fine. What it does allow is to for 'all' to display the command before the syntax error. If I had to make a choice I'd go the other way. Uh, what other way? However, I think with a little extra work it might be possible to have both. Right now, the way it is done, only a real syntax error skips logging. If you referenced an invalid table or something, it does print the log just before the invalid table name mention. How would we test the command type before hitting a syntax error? I can't think of a way, and I am not sure it would even be meaningful. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] logging statement levels
Bruce Momjian wrote: Right now we have log_min_error_statement: #log_min_error_statement = panic # Values in order of increasing severity: # debug5, debug4, debug3, debug2, debug1, # info, notice, warning, error, panic(off) which does allow control of printing only statements generating errors, which includes syntax errors. I don't see why this functionality should be mixed in with log_statement. Did you want a 'syntax error' level to log_statement, that would print only statements with syntax errors but not other errors? That doesn't seem very useful to me. It wasn't my idea, but I thought it was a good one. But it would go along with the idea of these settings as a list instead of a hierarchy, e.g.: log_statement = syntax-errors, ddl, mod In fact, I liked it so much that I thought syntax-errors should be the default instead of none. I think I'd prefer that to having it tied to the log_min_error_statement level. But I don't care that much. cheers andrew ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] logging statement levels
Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: Right now we have log_min_error_statement: #log_min_error_statement = panic # Values in order of increasing severity: # debug5, debug4, debug3, debug2, debug1, # info, notice, warning, error, panic(off) which does allow control of printing only statements generating errors, which includes syntax errors. I don't see why this functionality should be mixed in with log_statement. Did you want a 'syntax error' level to log_statement, that would print only statements with syntax errors but not other errors? That doesn't seem very useful to me. It wasn't my idea, but I thought it was a good one. But it would go along with the idea of these settings as a list instead of a hierarchy, e.g.: log_statement = syntax-errors, ddl, mod In fact, I liked it so much that I thought syntax-errors should be the default instead of none. I think I'd prefer that to having it tied to the log_min_error_statement level. But I don't care that much. OK, at least we understand each other. Right now we don't have any special syntax error log processing. We have errors logged through log_min_error_statement, and mod/ddl through the new log_statement. I can see a use case for having mod/ddl control of logging, and error control of logging, but why would you want to see syntax error queries but not other error queries? That's why I think log_min_error_statement is sufficient. If we add syntax logging,Thinks wouldn't that conflict with log_min_error_statement logging, because those are errors too. Maybe we need to add a 'synax' mode to log_min_error_statement above error that logs only syntax errors but not others. Thinks experiments yes, OK, I agree. Please forgive any denseness. Not sure if we need another level. Why do we have log_min_error_statement default to PANIC level? Wouldn't ERROR be a better default? cheers andrew ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] logging statement levels
Andrew Dunstan wrote: I think I'd prefer that to having it tied to the log_min_error_statement level. But I don't care that much. OK, at least we understand each other. Right now we don't have any special syntax error log processing. We have errors logged through log_min_error_statement, and mod/ddl through the new log_statement. I can see a use case for having mod/ddl control of logging, and error control of logging, but why would you want to see syntax error queries but not other error queries? That's why I think log_min_error_statement is sufficient. If we add syntax logging,Thinks wouldn't that conflict with log_min_error_statement logging, because those are errors too. Maybe we need to add a 'synax' mode to log_min_error_statement above error that logs only syntax errors but not others. Thinks experiments yes, OK, I agree. Please forgive any denseness. Not sure if we need another level. No problem. It is good to think through these things to make sure we have everything covered. Why do we have log_min_error_statement default to PANIC level? Wouldn't ERROR be a better default? Panic basically means off, meaning we don't print queries that generate errors. Should we print them by default? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
