I am not actively working on this now, but I'll come back to it for
PG12 if you or Lukas don't beat me to it, and I'll help/test/review if
I you do.  It seems there is plenty of demand for the feature and I'll
be very happy to see it.

Good to know, thanks!

I'd argue that it might be better to add a new argument to pg_plan_query()
and pg_plan_queries() and a new field to QueryDesc, i.e.:

PlannedStmt *
pg_plan_query(Query *querytree,
                          int cursorOptions,
                          ParamListInfo boundParams,
                          double *planningTime)

List *
pg_plan_queries(List *querytrees,
                                int cursorOptions,
                                ParamListInfo boundParams,
double *planningTime) /* total time as in
BuildCachedPlan() */

The measured time can later be passed to QueryDesc via PortalDefineQuery(). Of course, this requires more changes, but the result might be worth it.

What do you think?

So who does the actual timing work in this model?  Does the core code
do the timing, but it'd be disabled by default because NULL is usually
passed in, and you need to register an extension that provides a place
to stick the result in order to turn on the time-measuring code?  If
you mean that it's still the extension that does the timing, it seems
strange to have struct members and parameter arguments for something
specific that the core code doesn't understand.

Right, I guess my proposal was too brief. Yes, my idea's that the core code
should do the timing, which might be disabled by passing NULL to those
functions. However, given that there's lots of people out there who use
pg_stat_statements on a regular basis, it might be meaningful to just
turn it on unconditionally (my assumptions might be wrong, of course).
Alternatively, we could introduce a new GUC variable to disable this feature.

The extensions would no longer have to do the accounting.

As a more general version of that, I wondered about having some kind
of associative data structure (hash table, assoc list, etc) that would
somehow travel everywhere with the PlannedStmt, but not inside it,
just for the use of extensions.  Then planning time could be stored in
there by a planner hook, and the fished out by any other hook that
knows the same key (not sure how you manage that key space but I'm
sure you could come up with something).  That could have uses for
other extensions too, and could also be used for the "query ID", which
is, after all, the model that the abandoned planning_time member was
following.  Just a thought.

Perhaps we could change planner() so that it would return pointer to some struct holding PlannedStmt and a List of some nodes or structs for accounting.

--
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Reply via email to