<torikos...@oss.nttdata.com> wrote in
ISTM now that creating pg_stat_statements_xxx views
both for generic andcustom plans is better than my PoC patch.
On my second thought, it also makes pg_stat_statements too complicated
compared to what it makes possible..
I'm also worrying that whether taking generic and custom plan execution
time or not would be controlled by a GUC variable, and the default
would be not taking them.
Not many people will change the default.
Since the same queryid can contain various queries (different plan,
different parameter $n, etc.), I also started to feel that it is not
appropriate to get the execution time of only generic/custom queries
separately.
I suppose it would be normal practice to store past results of
pg_stat_statements for future comparisons.
If this is the case, I think that if we only add the number of
generic plan execution, it will give us a hint to notice the cause
of performance degradation due to changes in the plan between
generic and custom.
For example, if there is a clear difference in the number of times
the generic plan is executed between before and after performance
degradation as below, it would be natural to check if there is a
problem with the generic plan.
[after performance degradation]
=# SELECT query, calls, generic_calls FROM pg_stat_statements where
query like '%t1%';
query | calls | generic_calls
---------------------------------------------+-------+---------------
PREPARE p1 as select * from t1 where i = $1 | 1100 | 50
[before performance degradation]
=# SELECT query, calls, generic_calls FROM pg_stat_statements where
query like '%t1%';
query | calls | generic_calls
---------------------------------------------+-------+---------------
PREPARE p1 as select * from t1 where i = $1 | 1000 | 0
Attached a patch that just adds a generic call counter to
pg_stat_statements.
Any thoughts?
Regards,
--
Atsushi Torikoshidiff --git a/contrib/pg_stat_statements/pg_stat_statements--1.7--1.8.sql b/contrib/pg_stat_statements/pg_stat_statements--1.7--1.8.sql
index 0f63f08f7e..7fdef315ae 100644
--- a/contrib/pg_stat_statements/pg_stat_statements--1.7--1.8.sql
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.7--1.8.sql
@@ -44,7 +44,8 @@ CREATE FUNCTION pg_stat_statements(IN showtext boolean,
OUT blk_write_time float8,
OUT wal_records int8,
OUT wal_fpi int8,
- OUT wal_bytes numeric
+ OUT wal_bytes numeric,
+ OUT generic_calls int8
)
RETURNS SETOF record
AS 'MODULE_PATHNAME', 'pg_stat_statements_1_8'
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 72a117fc19..171c39f857 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -77,10 +77,12 @@
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/spin.h"
+#include "tcop/pquery.h"
#include "tcop/utility.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
+#include "utils/plancache.h"
#include "utils/timestamp.h"
PG_MODULE_MAGIC;
@@ -192,6 +194,7 @@ typedef struct Counters
int64 wal_records; /* # of WAL records generated */
int64 wal_fpi; /* # of WAL full page images generated */
uint64 wal_bytes; /* total amount of WAL generated in bytes */
+ int64 generic_calls; /* # of times generic plans executed */
} Counters;
/*
@@ -277,6 +280,9 @@ static int exec_nested_level = 0;
/* Current nesting depth of planner calls */
static int plan_nested_level = 0;
+/* Current plan type */
+static bool is_plan_type_generic = false;
+
/* Saved hook values in case of unload */
static shmem_startup_hook_type prev_shmem_startup_hook = NULL;
static post_parse_analyze_hook_type prev_post_parse_analyze_hook = NULL;
@@ -1034,6 +1040,20 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
*/
if (pgss_enabled(exec_nested_level) && queryDesc->plannedstmt->queryId != UINT64CONST(0))
{
+ /*
+ * Since ActivePortal is not available at ExecutorEnd, we preserve
+ * the plan type here.
+ */
+ Assert(ActivePortal);
+
+ if (ActivePortal->cplan)
+ {
+ if (ActivePortal->cplan->is_generic)
+ is_plan_type_generic = true;
+ else
+ is_plan_type_generic = false;
+ }
+
/*
* Set up to track total elapsed time in ExecutorRun. Make sure the
* space is allocated in the per-query context so it will go away at
@@ -1427,6 +1447,8 @@ pgss_store(const char *query, uint64 queryId,
e->counters.max_time[kind] = total_time;
e->counters.mean_time[kind] = total_time;
}
+ else if (kind == PGSS_EXEC && is_plan_type_generic)
+ e->counters.generic_calls += 1;
else
{
/*
@@ -1510,8 +1532,8 @@ pg_stat_statements_reset(PG_FUNCTION_ARGS)
#define PG_STAT_STATEMENTS_COLS_V1_1 18
#define PG_STAT_STATEMENTS_COLS_V1_2 19
#define PG_STAT_STATEMENTS_COLS_V1_3 23
-#define PG_STAT_STATEMENTS_COLS_V1_8 32
-#define PG_STAT_STATEMENTS_COLS 32 /* maximum of above */
+#define PG_STAT_STATEMENTS_COLS_V1_8 33
+#define PG_STAT_STATEMENTS_COLS 33 /* maximum of above */
/*
* Retrieve statement statistics.
@@ -1863,6 +1885,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
ObjectIdGetDatum(0),
Int32GetDatum(-1));
values[i++] = wal_bytes;
+ values[i++] = Int64GetDatumFast(tmp.generic_calls);
}
Assert(i == (api_version == PGSS_V1_0 ? PG_STAT_STATEMENTS_COLS_V1_0 :
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index 464bf0e5ae..8c5e304882 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -363,6 +363,14 @@
Total amount of WAL generated by the statement in bytes
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>generic_calls</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of times the statement was executed as generic plan
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index cc04b5b4be..d9f4523a7e 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -1219,10 +1219,12 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
plansource->total_custom_cost += cached_plan_cost(plan, true);
plansource->num_custom_plans++;
+ plan->is_generic = false;
}
else
{
plansource->num_generic_plans++;
+ plan->is_generic = true;
}
Assert(plan != NULL);
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 79d96e5ed0..5123ce252c 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -158,6 +158,7 @@ typedef struct CachedPlan
int generation; /* parent's generation number for this plan */
int refcount; /* count of live references to this struct */
MemoryContext context; /* context containing this CachedPlan */
+ bool is_generic; /* is this plan generic or not? */
} CachedPlan;
/*
