Hello,
At Tue, 27 Dec 2016 10:28:53 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20161227.102853.204155513.horiguchi.kyotaro@lab.ntt.co.jp>
> 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.
An additional comment on parser(planner?) part.
This patch make planner() copy the location and length from parse to result, but copying such stuffs is a job of
standard_planner.
Then the following is a comment on pg_stat_statements.c
- pg_stat_statements.c:977 - isParseNode(node) node should be parsenode.
- The name for the addional parameter query_loc is written as query_len in its prototype.
- In pgss_store, "else if (strlen(query)) != query_len)" doesn't work as expected because of one-off error. query_len
hereis the length of the query *excluding* the last semicolon.
- qtext_store doesn't rely on terminating nul character and the function already takes query length as a parameter. So,
there'sno need to copy the partial debug_query_string into palloc'ed memory. Just replacing the query with query_loc
willbe sufficient.
Have a nice holidays.
--
Kyotaro Horiguchi
NTT Open Source Software Center