2020-11-27 22:39 に Fujii Masao さんは書きました:
On 2020/11/27 21:39, Seino Yuki wrote:
2020-11-27 21:37 に Seino Yuki さんは書きました:
2020-11-16 12:28 に Seino Yuki さんは書きました:
Due to similar fixed, we have also merged the patches discussed in
the
following thread.
https://commitfest.postgresql.org/30/2736/
The patch is posted on the 2736 side of the thread.
Regards.
I forgot the attachment and will resend it.
The following patches have been committed and we have created a
compliant patch for them.
https://commitfest.postgresql.org/30/2736/
Please confirm.
Thanks for updating the patch! Here are review comments from me.
+ OUT reset_exec_time TIMESTAMP WITH TIME ZONE
I prefer "stats_reset" as the name of this column for the sake of
consistency with the similar column in other stats views like
pg_stat_database.
Isn't it better to use lower cases for "TIMESTAMP WITH TIME ZONE"
because other DDLs in pg_stat_statements do that for the declaraion
of data type?
@@ -565,6 +568,8 @@ pgss_shmem_startup(void)
pgss->n_writers = 0;
pgss->gc_count = 0;
pgss->stats.dealloc = 0;
+ pgss->stats.reset_exec_time = 0;
+ pgss->stats.reset_exec_time_isnull = true;
The reset time should be initialized with GetCurrentTimestamp() instead
of 0? If so, I think that we can completely get rid of
reset_exec_time_isnull.
+ memset(nulls, 0, sizeof(nulls));
If some entries in "values[]" may not be set, "values[]" also needs to
be initialized with 0.
MemSet() should be used, instead?
+ /* Read dealloc */
+ values[0] = stats.dealloc;
Int64GetDatum() should be used here?
+ reset_ts = GetCurrentTimestamp();
GetCurrentTimestamp() needs to be called only when all the entries
are removed. But it's called even in other cases.
Regards,
Thanks for the review!
Fixed the patch.
I learned a lot, especially since I didn't know about MemSet().
Regards.
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
index 2019a4ffe0..3504ca7eb1 100644
--- a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
@@ -5,9 +5,10 @@
--- Define pg_stat_statements_info
CREATE FUNCTION pg_stat_statements_info(
- OUT dealloc bigint
+ OUT dealloc bigint,
+ OUT stats_reset timestamp with time zone
)
-RETURNS bigint
+RETURNS record
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT VOLATILE PARALLEL SAFE;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 70cfdb2c9d..e90cd24f20 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -81,6 +81,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
+#include "utils/timestamp.h"
PG_MODULE_MAGIC;
@@ -199,6 +200,7 @@ typedef struct Counters
typedef struct pgssGlobalStats
{
int64 dealloc; /* # of times entries were deallocated */
+ TimestampTz stats_reset; /* timestamp with all stats reset */
} pgssGlobalStats;
/*
@@ -565,6 +567,7 @@ pgss_shmem_startup(void)
pgss->n_writers = 0;
pgss->gc_count = 0;
pgss->stats.dealloc = 0;
+ pgss->stats.stats_reset = GetCurrentTimestamp();
}
memset(&info, 0, sizeof(info));
@@ -1510,6 +1513,7 @@ pg_stat_statements_reset(PG_FUNCTION_ARGS)
#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_INFO_COLS 2
/*
* Retrieve statement statistics.
@@ -1889,7 +1893,18 @@ Datum
pg_stat_statements_info(PG_FUNCTION_ARGS)
{
pgssGlobalStats stats;
+ TupleDesc tupdesc;
+ HeapTuple result_tuple;
+ Datum values[PG_STAT_STATEMENTS_INFO_COLS];
+ bool nulls[PG_STAT_STATEMENTS_INFO_COLS];
+
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ elog(ERROR, "return type must be a row type");
+ tupdesc = BlessTupleDesc(tupdesc);
+
+ MemSet(values, 0, sizeof(values));
+ MemSet(nulls, 0, sizeof(nulls));
/* Read global statistics for pg_stat_statements */
{
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss;
@@ -1899,7 +1914,14 @@ pg_stat_statements_info(PG_FUNCTION_ARGS)
SpinLockRelease(&s->mutex);
}
- PG_RETURN_INT64(stats.dealloc);
+ /* Read dealloc */
+ values[0] = Int64GetDatum(stats.dealloc);
+
+ /* Read stats_reset */
+ values[1] = stats.stats_reset;
+
+ result_tuple = heap_form_tuple(tupdesc, values, nulls);
+ return HeapTupleGetDatum(result_tuple);
}
/*
@@ -2507,6 +2529,7 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid)
long num_entries;
long num_remove = 0;
pgssHashKey key;
+ TimestampTz reset_ts;
if (!pgss || !pgss_hash)
ereport(ERROR,
@@ -2553,12 +2576,14 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid)
num_remove++;
}
+ reset_ts = GetCurrentTimestamp();
/* Reset global statistics for pg_stat_statements */
{
volatile pgssSharedState *s = (volatile pgssSharedState *) pgss;
SpinLockAcquire(&s->mutex);
s->stats.dealloc = 0;
+ s->stats.stats_reset = reset_ts; /* reset execution time */
SpinLockRelease(&s->mutex);
}
}
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index 81915ea69b..821ae1f96e 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -523,6 +523,15 @@
<varname>pg_stat_statements.max</varname> were observed
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>reset_exec_time</structfield> <type>timestamp with time zone</type>
+ </para>
+ <para>
+ Shows the time at which <function>pg_stat_statements_reset(0,0,0)</function> was last called.
+ </para></entry>
+ </row>
+
</tbody>
</tgroup>
</table>
