Subject: Re: Patch: to pass query string to pg_plan_query()
To: legrand legrand <legrand_legrand@hotmail.com>,
 pgsql-hackers@postgresql.org
References: <1583789487074-0.post@n3.nabble.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <6ecbaaca-5a8e-5fe6-f7e1-a893e708bd7b@oss.nttdata.com>
Date: Thu, 26 Mar 2020 22:54:35 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0)
 Gecko/20100101 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <1583789487074-0.post@n3.nabble.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2020/03/10 6:31, legrand legrand wrote:
> Hello,
> 
> This is a call for committers, reviewers and users,
> regarding "planning counters in pg_stat_statements"
> patch [1] but not only.

Does anyone object to this patch? I'm thinking to commit it separetely
at first before committing the planning_counter_in_pg_stat_statements
patch.

> Historically, this version of pg_stat_statements
> with planning counters was performing 3 calls to
> pgss_store() for non utility statements in:
> 1 - pgss_post_parse_analyze (init entry with queryid
>      and store query text)
> 2 - pgss_planner_hook (to store planning counters)
> 3 - pgss_ExecutorEnd (to store execution counters)
> 
> Then a new version was proposed to remove one call
> to pgss_store() by adding the query string to the
> planner pg_plan_query():

But pgss_store() still needs to be called three times even in
non-utility command if the query has constants. Right?

> 1 - pgss_planner_hook (to store planning counters)
> 2 - pgss_ExecutorEnd (to store execution counters)
> 
> Many performances tests where performed concluding
> that there is no impact on this subject.

Sounds good!

> Patch "to pass query string to the planner", could be
> committed by itself, and (maybe) used by other extensions.
> 
> If this was done, this new version of pgss with planning
> counters could be committed as well, or even later
> (being used as a non core extension starting with pg13).
> 
> So please give us your feedback regarding this patch
> "to pass query string to the planner", if you have other
> use cases, or any comment regarding core architecture.

*As far as I heard*, pg_hint_plan extension uses very tricky way to
extract query string in the planner hook. So this patch would be
very helpful to make pg_hint_plan avoid using such tricky way.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters