Re: Exec statement logging

On Sat, 2005-05-14 at 16:55 -0400, Bruce Momjian wrote:
> Uh, I am confused by this.  Your code test is:
>
> +       if ((log_statement == LOGSTMT_ALL && save_log_duration) ||
> +             save_log_min_duration_statement)
> +               gettimeofday(&start_t, NULL);
>
> First, log_min_duration_statement == -1 turns off logging.  Your test
> would enable it for -1.  Also, you added "log_statement == LOGSTMT_ALL",
> but don't have a similar test lower down where gettimeofday is used, so
> I don't understand its usage here, or why it would be used in this
> place.  The original test is:
>
> +       if (save_log_duration || save_log_min_duration_statement != -1)
> +           gettimeofday(&start_t, NULL);

Yes, that looks wrong. Thanks for your diligence. I'm sorry that you've
had to both spot an error and recode for it, I was expecting to do that.

> One thing you did was to log debug_query_string, but I don't see how
> that could be the right value.  I assume it would be empty in a
> client-side execute, or be the previous query.  I instead used "EXECUTE
> portal_name" because that is what we are passed from the client.

I used the debug_query_string because even in the EXEC case it is set,
so that the SQL statement appears correctly in pg_stat_activity. It may
look wrong, but it is there.

That part, at least, is correct, since I have used the patch in tuning.

Perhaps it is only there when stats_command_string is set?

Setting the SQL string to be only EXECUTE portal_name makes the log
output almost useless for query tuning, so please reconsider that.
Perhaps you could include both the portal name and the SQL statement?

Best Regards, Simon Riggs