From 566a1d43cf6bfcc7f9385b581d98e07eab282cdd Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 29 Mar 2012 16:42:09 -0400 Subject: [PATCH] Improve contrib/pg_stat_statements' handling of PREPARE/EXECUTE statements. It's actually more useful for the module to ignore these. Ignoring EXECUTE (and not incrementing the nesting level) allows the executor hooks to charge the time to the underlying prepared query, which shows up as a stats entry with the original PREPARE as query string (possibly modified by suppression of constants, which might not be terribly useful here but it's not worth avoiding). This is much more useful than cluttering the stats table with a distinct entry for each textually distinct EXECUTE. Experimentation with this idea shows that it's also preferable to ignore PREPARE. If we don't, we get two stats table entries, one with the query string hash and one with the jumble-derived hash, but with the same visible query string (modulo those constants). This is confusing and not very helpful, since the first entry will only receive costs associated with initial planning of the query, which is not something counted at all normally by pg_stat_statements. (And if we do start tracking planning costs, we'd want them blamed on the other hash table entry anyway.) --- .../pg_stat_statements/pg_stat_statements.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 58c85ded58..178fdb9d4c 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -174,7 +174,7 @@ typedef struct pgssJumbleState /*---- Local variables ----*/ -/* Current nesting depth of ExecutorRun calls */ +/* Current nesting depth of ExecutorRun+ProcessUtility calls */ static int nested_level = 0; /* Saved hook values in case of unload */ @@ -773,7 +773,22 @@ pgss_ProcessUtility(Node *parsetree, const char *queryString, ParamListInfo params, bool isTopLevel, DestReceiver *dest, char *completionTag) { - if (pgss_track_utility && pgss_enabled()) + /* + * If it's an EXECUTE statement, we don't track it and don't increment + * the nesting level. This allows the cycles to be charged to the + * underlying PREPARE instead (by the Executor hooks), which is much more + * useful. + * + * We also don't track execution of PREPARE. If we did, we would get one + * hash table entry for the PREPARE (with hash calculated from the query + * string), and then a different one with the same query string (but hash + * calculated from the query tree) would be used to accumulate costs of + * ensuing EXECUTEs. This would be confusing, and inconsistent with other + * cases where planning time is not included at all. + */ + if (pgss_track_utility && pgss_enabled() && + !IsA(parsetree, ExecuteStmt) && + !IsA(parsetree, PrepareStmt)) { instr_time start; instr_time duration;