Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements

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.