Обсуждение: 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 Index: src/backend/tcop/pquery.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/tcop/pquery.c,v retrieving revision 1.74 diff -c -c -r1.74 pquery.c *** src/backend/tcop/pquery.c 29 Nov 2003 19:51:57 -0000 1.74 --- src/backend/tcop/pquery.c 4 Mar 2004 18:29:03 -0000 *************** *** 406,411 **** --- 406,415 ---- if (completionTag) completionTag[0] = '\0'; + /* PORTAL_MULTI_QUERY logs its own stats per query */ + if (log_executor_stats && portal->strategy != PORTAL_MULTI_QUERY) + ResetUsage(); + /* * Check for improper portal use, and mark portal active. */ *************** *** 499,504 **** --- 503,511 ---- PortalContext = savePortalContext; QueryContext = saveQueryContext; + + if (log_executor_stats && portal->strategy != PORTAL_MULTI_QUERY) + ShowUsage("EXECUTOR STATISTICS"); return result; }
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