Fix for log_executor_stats
The log_executor_stats config variable doesn't work 100%. It shows
stats for only certain SQL queries. Looking at utils/portal.h:
* We have several execution strategies for Portals, depending on what
* query or queries are to be executed. (Note: in all cases, a Portal
* executes just a single source-SQL query, and thus produces just a
* single result from the user's viewpoint. However, the rule rewriter
* may expand the single source query to zero or many actual queries.)
*
* PORTAL_ONE_SELECT: the portal contains one single SELECT query. We run
* the Executor incrementally as results are demanded. This strategy also
* supports holdable cursors (the Executor results can be dumped into a
* tuplestore for access after transaction completion).
*
* PORTAL_UTIL_SELECT: the portal contains a utility statement that returns
* a SELECT-like result (for example, EXPLAIN or SHOW). On first execution,
* we run the statement and dump its results into the portal tuplestore;
* the results are then returned to the client as demanded.
*
* PORTAL_MULTI_QUERY: all other cases. Here, we do not support partial
* execution: the portal's queries will be run to completion on first call.
it only logs executor stats for PORTAL_MULTI_QUERY queries. I assume
this was done so that individual queries are logged rather than the
entire multi-query. However, the code shows no executor stats for
non-multi queries, which is certainly a bug. Even a simple "SELECT col
FROM tab" or "SELECT 1" emits no statistics.
This used to work in the past, but it appears broken in 7.4.X too. I
guess I should fix it there too, though obviously very few people use
this feature or we would have gotten reports.
The attached fix keeps the multi-query stats code unchanged, but adds
stats for non-multi-query cases.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Attachments:
/pgpatches/exectext/plainDownload+7-0
Bruce Momjian <pgman@candle.pha.pa.us> writes:
it only logs executor stats for PORTAL_MULTI_QUERY queries. I assume
this was done so that individual queries are logged rather than the
entire multi-query.
I think it was just an oversight. IIRC, the PORTAL_MULTI_QUERY path of
control is the only one that has direct predecessor code in pre-7.4
releases. I probably missed the need to add stats code to the new code
paths.
Are you sure that log_executor_stats is the only missing case?
Also, I am not sure that the code paths for "extended query" messages
are doing logging correctly. I recall being worried about how to
translate the logging definitions into situations where parsing and
execution are separate, or where a statement may be partially executed
and then we come back for more. I didn't have time in the 7.4 cycle to
really think about it, but someone should take a look.
regards, tom lane
Tom Lane wrote:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
it only logs executor stats for PORTAL_MULTI_QUERY queries. I assume
this was done so that individual queries are logged rather than the
entire multi-query.I think it was just an oversight. IIRC, the PORTAL_MULTI_QUERY path of
control is the only one that has direct predecessor code in pre-7.4
releases. I probably missed the need to add stats code to the new code
paths.
OK, patch applied.
Are you sure that log_executor_stats is the only missing case?
You are asking if other things are in the Multi path but not in the
non-multi path. The only thing I saw was this from PortalRunMulti:
/*
* If a command completion tag was supplied, use it. Otherwise use
* the portal's commandTag as the default completion tag.
*
* Exception: clients will expect INSERT/UPDATE/DELETE tags to have
* counts, so fake something up if necessary. (This could happen if
* the original query was replaced by a DO INSTEAD rule.)
*/
if (completionTag && completionTag[0] == '\0')
{
if (portal->commandTag)
strcpy(completionTag, portal->commandTag);
if (strcmp(completionTag, "INSERT") == 0)
strcpy(completionTag, "INSERT 0 0");
else if (strcmp(completionTag, "UPDATE") == 0)
strcpy(completionTag, "UPDATE 0");
else if (strcmp(completionTag, "DELETE") == 0)
strcpy(completionTag, "DELETE 0");
}
Oh, I see this too:
ereport(DEBUG3,
(errmsg_internal("ProcessQuery")));
I will duplicate that one in the right place.
Also, I am not sure that the code paths for "extended query" messages
are doing logging correctly. I recall being worried about how to
translate the logging definitions into situations where parsing and
execution are separate, or where a statement may be partially executed
and then we come back for more. I didn't have time in the 7.4 cycle to
really think about it, but someone should take a look.
How do we test that?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073