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?