On Sun, Mar 24, 2019 at 11:24 AM Julien Rouhaud <rjuju...@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.
