From 6c738707abdd72807ec94dbafd346f077e482f74 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Fri, 4 Dec 2020 13:33:51 +0800 Subject: [PATCH v1] Add a bool toplevel column to pg_stat_statements. As top level statements include resource consumption for underlying statements, it's not possible to compute the total resource consumption accurately. Fix that by adding a new toplevel boolean field that indicates whether the counters were cumulated for queries executed at top level or not. It can lead to more entries being stored for the same workload if pg_stat_statements.track is set to all, but it seems unlikely to have the same queries being executed both at top level and as nested statements. --- contrib/pg_stat_statements/Makefile | 3 +- .../expected/pg_stat_statements.out | 40 +++++++++++++ .../pg_stat_statements--1.9--1.10.sql | 57 +++++++++++++++++++ .../pg_stat_statements/pg_stat_statements.c | 42 +++++++++++--- .../pg_stat_statements.control | 2 +- .../sql/pg_stat_statements.sql | 21 +++++++ doc/src/sgml/pgstatstatements.sgml | 9 +++ 7 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile index 3ec627b956..cab4f626ad 100644 --- a/contrib/pg_stat_statements/Makefile +++ b/contrib/pg_stat_statements/Makefile @@ -6,7 +6,8 @@ OBJS = \ pg_stat_statements.o EXTENSION = pg_stat_statements -DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \ +DATA = pg_stat_statements--1.4.sql \ + pg_stat_statements--1.9--1.10.sql pg_stat_statements--1.8--1.9.sql \ pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \ pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \ pg_stat_statements--1.3--1.4.sql pg_stat_statements--1.2--1.3.sql \ diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out index 16158525ca..fb97f68737 100644 --- a/contrib/pg_stat_statements/expected/pg_stat_statements.out +++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out @@ -876,4 +876,44 @@ SELECT dealloc FROM pg_stat_statements_info; 0 (1 row) +-- +-- top level handling +-- +SET pg_stat_statements.track = 'top'; +DELETE FROM test; +DO $$ +BEGIN + DELETE FROM test; +END; +$$ LANGUAGE plpgsql; +SELECT query, toplevel, plans, calls FROM pg_stat_statements WHERE query LIKE '%DELETE%' ORDER BY query COLLATE "C", toplevel; + query | toplevel | plans | calls +-----------------------+----------+-------+------- + DELETE FROM test | t | 1 | 1 + DO $$ +| t | 0 | 1 + BEGIN +| | | + DELETE FROM test;+| | | + END; +| | | + $$ LANGUAGE plpgsql | | | +(2 rows) + +SET pg_stat_statements.track = 'all'; +DELETE FROM test; +DO $$ +BEGIN + DELETE FROM test; +END; +$$ LANGUAGE plpgsql; +SELECT query, toplevel, plans, calls FROM pg_stat_statements WHERE query LIKE '%DELETE%' ORDER BY query COLLATE "C", toplevel; + query | toplevel | plans | calls +-----------------------+----------+-------+------- + DELETE FROM test | f | 1 | 1 + DELETE FROM test | t | 2 | 2 + DO $$ +| t | 0 | 2 + BEGIN +| | | + DELETE FROM test;+| | | + END; +| | | + $$ LANGUAGE plpgsql | | | +(3 rows) + DROP EXTENSION pg_stat_statements; diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql b/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql new file mode 100644 index 0000000000..f97d16497d --- /dev/null +++ b/contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql @@ -0,0 +1,57 @@ +/* contrib/pg_stat_statements/pg_stat_statements--1.9--1.10.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.10'" to load this file. \quit + +/* First we have to remove them from the extension */ +ALTER EXTENSION pg_stat_statements DROP VIEW pg_stat_statements; +ALTER EXTENSION pg_stat_statements DROP FUNCTION pg_stat_statements(boolean); + +/* Then we can drop them */ +DROP VIEW pg_stat_statements; +DROP FUNCTION pg_stat_statements(boolean); + +/* Now redefine */ +CREATE FUNCTION pg_stat_statements(IN showtext boolean, + OUT userid oid, + OUT dbid oid, + OUT toplevel bool, + OUT queryid bigint, + OUT query text, + OUT plans int8, + OUT total_plan_time float8, + OUT min_plan_time float8, + OUT max_plan_time float8, + OUT mean_plan_time float8, + OUT stddev_plan_time float8, + OUT calls int8, + OUT total_exec_time float8, + OUT min_exec_time float8, + OUT max_exec_time float8, + OUT mean_exec_time float8, + OUT stddev_exec_time float8, + OUT rows int8, + OUT shared_blks_hit int8, + OUT shared_blks_read int8, + OUT shared_blks_dirtied int8, + OUT shared_blks_written int8, + OUT local_blks_hit int8, + OUT local_blks_read int8, + OUT local_blks_dirtied int8, + OUT local_blks_written int8, + OUT temp_blks_read int8, + OUT temp_blks_written int8, + OUT blk_read_time float8, + OUT blk_write_time float8, + OUT wal_records int8, + OUT wal_fpi int8, + OUT wal_bytes numeric +) +RETURNS SETOF record +AS 'MODULE_PATHNAME', 'pg_stat_statements_1_10' +LANGUAGE C STRICT VOLATILE PARALLEL SAFE; + +CREATE VIEW pg_stat_statements AS + SELECT * FROM pg_stat_statements(true); + +GRANT SELECT ON pg_stat_statements TO PUBLIC; diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 70cfdb2c9d..c79e60bfde 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -124,7 +124,8 @@ typedef enum pgssVersion PGSS_V1_1, PGSS_V1_2, PGSS_V1_3, - PGSS_V1_8 + PGSS_V1_8, + PGSS_V1_10 } pgssVersion; typedef enum pgssStoreKind @@ -145,17 +146,13 @@ typedef enum pgssStoreKind /* * Hashtable key that defines the identity of a hashtable entry. We separate * queries by user and by database even if they are otherwise identical. - * - * Right now, this structure contains no padding. If you add any, make sure - * to teach pgss_store() to zero the padding bytes. Otherwise, things will - * break, because pgss_hash is created using HASH_BLOBS, and thus tag_hash - * is used to hash this. */ typedef struct pgssHashKey { Oid userid; /* user OID */ Oid dbid; /* database OID */ uint64 queryid; /* query identifier */ + bool toplevel; /* query executed at top level */ } pgssHashKey; /* @@ -335,6 +332,7 @@ PG_FUNCTION_INFO_V1(pg_stat_statements_reset_1_7); PG_FUNCTION_INFO_V1(pg_stat_statements_1_2); PG_FUNCTION_INFO_V1(pg_stat_statements_1_3); PG_FUNCTION_INFO_V1(pg_stat_statements_1_8); +PG_FUNCTION_INFO_V1(pg_stat_statements_1_10); PG_FUNCTION_INFO_V1(pg_stat_statements); PG_FUNCTION_INFO_V1(pg_stat_statements_info); @@ -1327,9 +1325,11 @@ pgss_store(const char *query, uint64 queryId, } /* Set up key for hashtable search */ + memset(&key, 0, sizeof(pgssHashKey)); key.userid = GetUserId(); key.dbid = MyDatabaseId; key.queryid = queryId; + key.toplevel = (exec_nested_level == 0); /* Lookup the hash table entry with shared lock. */ LWLockAcquire(pgss->lock, LW_SHARED); @@ -1509,7 +1509,8 @@ pg_stat_statements_reset(PG_FUNCTION_ARGS) #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_10 33 +#define PG_STAT_STATEMENTS_COLS 33 /* maximum of above */ /* * Retrieve statement statistics. @@ -1521,6 +1522,16 @@ pg_stat_statements_reset(PG_FUNCTION_ARGS) * expected API version is identified by embedding it in the C name of the * function. Unfortunately we weren't bright enough to do that for 1.1. */ +Datum +pg_stat_statements_1_10(PG_FUNCTION_ARGS) +{ + bool showtext = PG_GETARG_BOOL(0); + + pg_stat_statements_internal(fcinfo, PGSS_V1_10, showtext); + + return (Datum) 0; +} + Datum pg_stat_statements_1_8(PG_FUNCTION_ARGS) { @@ -1640,6 +1651,10 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, if (api_version != PGSS_V1_8) elog(ERROR, "incorrect number of output arguments"); break; + case PG_STAT_STATEMENTS_COLS_V1_10: + if (api_version != PGSS_V1_10) + elog(ERROR, "incorrect number of output arguments"); + break; default: elog(ERROR, "incorrect number of output arguments"); } @@ -1731,6 +1746,8 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, values[i++] = ObjectIdGetDatum(entry->key.userid); values[i++] = ObjectIdGetDatum(entry->key.dbid); + if (api_version >= PGSS_V1_10) + values[i++] = BoolGetDatum(entry->key.toplevel); if (is_allowed_role || entry->key.userid == userid) { @@ -1868,6 +1885,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, api_version == PGSS_V1_2 ? PG_STAT_STATEMENTS_COLS_V1_2 : api_version == PGSS_V1_3 ? PG_STAT_STATEMENTS_COLS_V1_3 : api_version == PGSS_V1_8 ? PG_STAT_STATEMENTS_COLS_V1_8 : + api_version == PGSS_V1_10 ? PG_STAT_STATEMENTS_COLS_V1_10 : -1 /* fail if you forget to update this assert */ )); tuplestore_putvalues(tupstore, tupdesc, values, nulls); @@ -2519,9 +2537,19 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid) if (userid != 0 && dbid != 0 && queryid != UINT64CONST(0)) { /* If all the parameters are available, use the fast path. */ + memset(&key, 0, sizeof(pgssHashKey)); key.userid = userid; key.dbid = dbid; key.queryid = queryid; + key.toplevel = false; + + /* Remove the key if exists */ + entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_REMOVE, NULL); + if (entry) /* found */ + num_remove++; + + /* Also remove entries for top level statements */ + key.toplevel = true; /* Remove the key if exists */ entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_REMOVE, NULL); diff --git a/contrib/pg_stat_statements/pg_stat_statements.control b/contrib/pg_stat_statements/pg_stat_statements.control index 2f1ce6ed50..0747e48138 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.control +++ b/contrib/pg_stat_statements/pg_stat_statements.control @@ -1,5 +1,5 @@ # pg_stat_statements extension comment = 'track planning and execution statistics of all SQL statements executed' -default_version = '1.9' +default_version = '1.10' module_pathname = '$libdir/pg_stat_statements' relocatable = true diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql index 6f58d9d0f6..56d8526ccf 100644 --- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql +++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql @@ -364,4 +364,25 @@ SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query COLLATE SELECT pg_stat_statements_reset(); SELECT dealloc FROM pg_stat_statements_info; +-- +-- top level handling +-- +SET pg_stat_statements.track = 'top'; +DELETE FROM test; +DO $$ +BEGIN + DELETE FROM test; +END; +$$ LANGUAGE plpgsql; +SELECT query, toplevel, plans, calls FROM pg_stat_statements WHERE query LIKE '%DELETE%' ORDER BY query COLLATE "C", toplevel; + +SET pg_stat_statements.track = 'all'; +DELETE FROM test; +DO $$ +BEGIN + DELETE FROM test; +END; +$$ LANGUAGE plpgsql; +SELECT query, toplevel, plans, calls FROM pg_stat_statements WHERE query LIKE '%DELETE%' ORDER BY query COLLATE "C", toplevel; + DROP EXTENSION pg_stat_statements; diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index 81915ea69b..1ad47ce97c 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -79,6 +79,15 @@ + + + toplevel bool + + + True if the query was executed as a top level statement + + + queryid bigint -- 2.20.1