Re: Planning counters in pg_stat_statements (using pgss_store)

On Sun, Mar 24, 2019 at 11:24 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > there are 4 tests to check if planning_time is zero or not, it's quite
> > > messy.  Could you refactor the code to avoid so many tests?  It would
> > > probably be useful to add some asserts to check that we don't provide
> > > both planning_time == 0 and execution related values.  The function's
> > > comment would also need to be adapted to mention the new rationale
> > > with planning_time.
> >
> > Fixed
>
> +       /* updating counters for execute OR planning */
> +       Assert(planning_time > 0 && total_time > 0);
> +       if (planning_time == 0)
>
> This is obviously incorrect.  The general sanity check for exclusion
> between planning_time and total_time should be at the beginning of
> pgss_store.  Maybe some others asserts are needed to verify that
> planning_time  cannot be provided along jstate or other conditions.

Actually, since pgss_store is now called to either:

- explicitly store a query text
- accumulate planning duration
- accumulate execution duration

and they're all mutually exclusive, It's probably better to change
pgss_store to pass an enum to describe what the call is for , and keep
a single time parameter.  It should make the code simpler.