Re: defer statement logging until after parse

The problem I see with this patch is that it doesn't print the error
query on a syntax error. That seems wrong.

I think you should print the query before parsing if they are asking for
all queries to be logged, and print them after parsing if they want only
DDL or DDL and data modification queries. I realize that duplicates
some function calls, but I don't see any other way.

---------------------------------------------------------------------------

Andrew Dunstan wrote:
>
> The attached patch is for review, not application. It defers logging
> statements until after they have been parsed. There should be no
> observable difference in behaviour if there is a successful parse, and
> on error the following traces appear:
>
> line:3 ERROR: syntax error at or near "se3d2" at character 1
> line:4 LOG: parse error in statement: se3d2 aaa;
>
> Basically, I want to know that this will not break anything, and if so I
> will use it as the basis for a selective statement logging facility,
> based on the command tag(s) of the parsed statement(s), and
> incorporating Greg Stark's suggesion of a "syntax error" logging level.
>
> cheers
>
> andrew
>

> Index: src/backend/tcop/postgres.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
> retrieving revision 1.394
> diff -c -r1.394 postgres.c
> *** src/backend/tcop/postgres.c 9 Mar 2004 04:43:07 -0000 1.394
> --- src/backend/tcop/postgres.c 11 Mar 2004 16:31:10 -0000
> ***************
> *** 118,123 ****
> --- 118,128 ----
> static bool ignore_till_sync = false;
>
> /*
> + * place to save the input pointer in case of a parse error
> + */
> + static char *parse_input_string = NULL;
> +
> + /*
> * If an unnamed prepared statement exists, it's stored here.
> * We keep it separate from the hashtable kept by commands/prepare.c
> * in order to reduce overhead for short-lived queries.
> ***************
> *** 462,476 ****
> {
> 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");
>
> --- 467,487 ----
> {
> List *raw_parsetree_list;
>
> if (log_parser_stats)
> ResetUsage();
>
> + parse_input_string = query_string;
> +
> raw_parsetree_list = raw_parser(query_string);
>
> + /* if we get here there was no parse error */
> +
> + parse_input_string = NULL;
> +
> + if (log_statement)
> + ereport(LOG,
> + (errmsg("statement: %s", query_string)));
> +
> if (log_parser_stats)
> ShowUsage("PARSER STATISTICS");
>
> ***************
> *** 2747,2752 ****
> --- 2758,2775 ----
> send_rfq = true; /* initially, or after error */
>
> /*
> + * if parse_input_string is not null, we must have got here through a
> + * parser error, in which case we log it if appropriate.
> + */
> +
> + if (log_statement && parse_input_string != NULL)
> + ereport(LOG,
> + (errmsg("parse error in statement: %s", parse_input_string)));
> +
> + parse_input_string = NULL;
> +
> +
> + /*
> * Non-error queries loop here.
> */
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073