Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Andrei Zubkov <zubkov(at)moonset(dot)ru>
Cc: Greg Stark <stark(at)mit(dot)edu>, "Anton A(dot) Melnikov" <aamelnikov(at)inbox(dot)ru>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements
Date: 2022-03-30 09:31:41
Message-ID: 20220330093141.li67uzuhys3xair6@jrouhaud
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On Fri, Mar 25, 2022 at 01:25:23PM +0300, Andrei Zubkov wrote:
> Greg, thank you for your attention and for your thought.
> I've just completed the 6th version of a patch implementing idea
> proposed by Julien Rouhaud, i.e. without auxiliary statistics. 6th
> version will reset current min/max fields to zeros until the first plan
> or execute.


I've decided to use zeros here because planning statistics
> is zero in case of disabled tracking. I think sampling solution could
> easily handle this.

I'm fine with it. It's also consistent with the planning counters when
track_planning is disabled. And even if the sampling solution doesn't handle
it, you will simply get consistent values, like "0 calls with minmax timing of
0 msec", so it's not really a problem.

Feature wise, I'm happy with the patch. I just have a few comments.


- it's missing some test in sql/oldextversions.sql to validate that the code
works with the extension in version 1.9
- the last test removed the minmax_plan_zero field, why?


+ TimestampTz stats_since; /* timestamp of entry allocation moment */

I think "timestamp of entry allocation" is enough?

+ * Calculate min and max time. min = 0 and max = 0
+ * means that min/max statistics reset was happen

maybe "means that the min/max statistics were reset"

+ * Reset min/max values of specified entries
+ */
+static void
+entry_minmax_reset(Oid userid, Oid dbid, uint64 queryid)

There's a lot of duplicated logic with entry_reset().
Would it be possible to merge at least the C reset function to handle either
all-metrics or minmax-only? Also, maybe it would be better to have a single SQL
reset function, something like:

pg_stat_statements_reset(IN userid Oid DEFAULT 0,
IN dbid Oid DEFAULT 0,
IN queryid bigint DEFAULT 0,
IN minmax_only DEFAULT false


+ <structfield>stats_since</structfield> <type>timestamp with time zone</type>
+ </para>
+ <para>
+ Timestamp of statistics gathering start for the statement

The description is a bit weird. Maybe like "Time at which statistics gathering
started for this statement"? Same for the minmax version.

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-03-30 10:59:47 Re: Identify missing publications from publisher while create/alter subscription.
Previous Message Amit Kapila 2022-03-30 08:59:56 Re: Logical replication timeout problem