Hi, On Fri, Apr 01, 2022 at 11:38:52AM -0400, Greg Stark wrote: > FYI this has a compiler warning showing up on the cfbot: > > [13:19:51.544] pg_stat_statements.c: In function ‘entry_reset’: > [13:19:51.544] pg_stat_statements.c:2598:32: error: > ‘minmax_stats_reset’ may be used uninitialized in this function > [-Werror=maybe-uninitialized] > [13:19:51.544] 2598 | entry->minmax_stats_since = minmax_stats_reset; > [13:19:51.544] | ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~ > > If the patch is otherwise ready to commit then this is an issue that > should be fixed before marking it ready to commit. > > Given that this is the last week before feature freeze it'll probably > get moved to a future commitfest unless it's ready to commit.
As I mentioned in my last review I think feature wise the patch is ok, it just needed a few minor changes. It's a small patch but can help *a lot* tools on top of pg_stat_statements and give users a better overview of their workload so it would be nice to commit it in v15. I was busy looking at the prefetch patch today (not done yet), but I plan to review the last version over the weekend. After a quick look at the patch it seems like a compiler bug. I'm not sure which clang version is used, but can't reproduce it locally using clang 13. I already saw similar false positive, when a variable is initialized in a branch (here minmax_only == true), and only then used in similar branches. I guess that pg_stat_statement_reset() is so expensive that an extra gettimeofday() wouldn't change much. Otherwise initializing to NULL should be enough.
