Re: [HACKERS] Planning counters in pg_stat_statements

From: Dmitry Ivanov <d(dot)ivanov(at)postgrespro(dot)ru>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Lukas Fittl <lukas(at)fittl(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, legrand legrand <legrand_legrand(at)hotmail(dot)com>
Subject: Re: [HACKERS] Planning counters in pg_stat_statements
Date: 2018-05-16 08:27:09
Message-ID: e07dabf18b2f408230dd3b9fe0bf558e@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Rofail 2018-05-16 08:31:50 Re: [HACKERS] GSoC 2017: Foreign Key Arrays
Previous Message Dean Rasheed 2018-05-16 07:15:33 Re: NaNs in numeric_power (was Re: Postgres 11 release notes)