Re: Exec statement logging

Simon Riggs wrote:
> Following patch is a minor addition to postgres.c that allows the two
> existing statement logging techniques to work with V3 exec. This then
> allows statement logging with PostgreSQL 8.0+ for JDBC and other V3
> connection types.
>
> The rationale of this patch is to add functionality without modifying
> existing behaviour. There is expected to be some difficulty with
> log_statement producing a log line at both parse and exec, but some may
> find that useful. Since there are two ways of producing statement
> logging with duration times, setting log_min_duration_statement=0 will
> avoid the logging of statements for both parse and exec. For many this
> method is already the preferred way of logging statement performance
> stats.

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);

> There is no attempt to log parameters, since these are not often
> required for performance analysis.

OK.

> The enclosed patch has been tested against cvstip.
>
> I also see this as a backpatch onto 8.0, since it prevents statements
> from being logged as described in the manual and prevents effective
> performance tuning. It has not been tested against 8.0.2, though was
> originally written against 8.0.1 and is believed to apply cleanly.

I don't think it should be backpatched. This is a behavior changes that
can be seen as a feature addition.

> Some code has been duplicated with this patch; refactoring and cleanup
> can be performed should anybody desire it.

Not sure it is worth it considering how many variables are involved.

> The patch was produced quickly to assist tuning efforts during
> Scalability & Performance benchmarking of PostgreSQL 8.0 carried out at
> Unisys Corporation's Mission Viejo engineering laboratory. The
> development was sponsored by Unisys Corporation and the patch has now
> been donated to the PostgreSQL community under the standard
> PostgreSQL/BSD licence. Approval for release of this code has been given
> in writing to me by the Director, Open Runtime Products, Unisys on April
> 8, 2005.

I have made a new version of the patch using your patch only as a guide.
I copied the sections you suggested. It compiles fine but I would like
someone to test that it actually works for client-side EXECUTE. I don't
have a test setup for that here.

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.

--
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