Re: [HACKERS] Planning counters in pg_stat_statements - Mailing list pgsql-hackers

From Dmitry Ivanov
Subject Re: [HACKERS] Planning counters in pg_stat_statements
Date
Msg-id e07dabf18b2f408230dd3b9fe0bf558e@postgrespro.ru
Whole thread Raw
In response to Re: [HACKERS] Planning counters in pg_stat_statements  (Thomas Munro <thomas.munro@enterprisedb.com>)
List pgsql-hackers
> 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


pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: NaNs in numeric_power (was Re: Postgres 11 release notes)
Next
From: Mark Rofail
Date:
Subject: Re: [HACKERS] GSoC 2017: Foreign Key Arrays