Re: BUG: pg_stat_statements query normalization issues with combined queries

Fabien COELHO <coelho@cri.ensmp.fr>

From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, craig.ringer@2ndquadrant.com, robertmhaas@gmail.com, PostgreSQL Developers <pgsql-hackers@postgresql.org>
Date: 2016-12-30T14:10:42Z
Lists: pgsql-hackers

Attachments

Hello,

>> Yeah, that's what I was thinking of.  There aren't very many places that
>> would need to know about that, I believe; [...]
>
> For fixing the information in pg_stat_statement, the location data must be 
> transported from the parsed node to the query to the planned node, because 
> the later two nodes types are passed to different hooks.
>
> Now the detail is that utility statements, which seems to be nearly all of 
> them but select/update/delete/insert, do not have plans: The statement itself 
> is its own plan... so there is no place to store the location & length.

Here is an updated version:

Changes wrt v2:

  - I have added the missing stuff under /nodes/, this is stupid code that
    should be automatically generated:-(

  - I have added comments in "gram.y" about how the length is computed.
    I have also slightly simplified the rule code there.

  - I have rename "location" in create table space to "location_dir"
    to avoid confusion.

  - I have renamed the fields "location" and "length" instead of q*.

  - I have moved the location & lenth copies in standard_planner.

  - I have fixed the function declaration typo.

  - I have simplified pgss_store code to avoid a copy, and move the
    length truncation in qtext_store.

  - I have improved again the pg_stat_statement regression tests with
    combined utility statement tests, which implied some fixes to
    extract the right substring for utility queries.

However, not changed:

  - I cannot use the intermediate node trick suggested by Tom because
    it does not work for utility statements which do not have plans, so
    the code still adds location & length, sorry.

  - I still use the 'last_semicolon' lexer variable. The alternative is to
    change rules so as not to skip empty statements, then write a loop to
    compute the length based on successor location, and remove the empty
    statements. It can be done, I do not think it is better, it is only
    different and more verbose. I'll do it if required by a committer.

-- 
Fabien.

Commits

  1. Teach contrib/pg_stat_statements to handle multi-statement commands better.

  2. Change representation of statement lists, and add statement location info.

  3. Revert my bad decision of about a year ago to make PortalDefineQuery