Re: BUG: pg_stat_statements query normalization issues with combined queries

Hello,

At Mon, 26 Dec 2016 16:00:57 +0100 (CET), Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote in <alpine(dot)DEB(dot)2(dot)20(dot)1612261554510(dot)4911(at)lancre>
>
> Hello Craig,
>
> > Please put me (ringerc) down as a reviewer.
>
> Done.
>
> Please do not loose time reviewing stupid version 1... skip to version
> 2 directly:-)
>
> Also, note that the query-location part may be considered separately
> from the pg_stat_statements part which uses it.

In nonterminal stmtmulti, setQueryLocation is added and used to
calcualte and store the length of every stmt's. This needs an
extra storage in bse_yy_extra_type and introduces a bit
complicated stuff. But I think raw_parser can do that without
such extra storage and labor, then gram.y gets far simpler.

The struct member to store the location and length is named
'qlocation', and 'qlength' in the added ParseNode. But many nodes
already have 'location' of exactly the same purpopse. I don't see
a necessity to differentiate them.

Putting the two above together, the following is my suggestion
for the parser part.

- Make all parse nodes have the following two members at the
beginning. This unifies all parse node from the view of setting
their location. Commenting about this would be better.

| NodeTag type;
| int location;

- Remove struct ParseNode and setQueryLocation. stmtmulti sets
location in the same manner to other kind of nodes, just doing
'= @n'. base_yyparse() doesn't calculate statement length.

- Make raw_parser caluclate statement length then store it into
each stmt scanning the yyextra.parsetree.

What do you think about this?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center