Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

On Sat, Apr 02, 2022 at 01:12:54PM +0300, Andrei Zubkov wrote:
> On Fri, 2022-04-01 at 13:01 -0700, Andres Freund wrote:
> > It seems decidedly not great to have four copies of this code. It was
> > already
> > not great before, but this patch makes the duplicated section go from
> > four
> > lines to 20 or so.
> 
> Agreed. I've created the single_entry_reset() function wrapping this
> code. I wonder if it should be declared as inline to speedup a little.

Maybe a macro would be better here?  I don't know if that's generally ok or
just old and not-that-great code, but there are other places relying on macros
when a plain function call isn't that convenient (like here returning 0 or 1 as
a hack for incrementing num_remove), for instance in hba.c.

> On Sat, 2022-04-02 at 15:21 +0800, Julien Rouhaud wrote:
> > I'm not sure about returning the ts.  If you need it you could call
> > SELECT
> > now() FROM pg_stat_statements_reset() (or clock_timestamp()).  It
> > won't be
> > entirely accurate but since the function will have an exclusive lock
> > during the
> > whole execution that shouldn't be a problem.  Now you're already
> > adding a new
> > version of the C function so I guess that it wouldn't require any
> > additional
> > effort so why not.
> 
> I think that if we can do it in accurate way and there is no obvious
> side effects, why not to try it...
> Changing of pg_stat_statements_reset function result caused a
> confiderable tests update. Also, I'm not sure that my description of
> this feature in the docs is blameless..
> 
> After all, I'm a little bit in doubt about this feature, so I'm ready
> to rollback it.

Well, I personally don't think that I would need it for powa as it's designed
to have very frequent snapshot.  I know you have a different approach in
pg_profile, but I'm not sure it will be that useful for you either?