Hello, At Tue, 27 Dec 2016 10:28:53 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote in <20161227.102853.204155513.horiguchi.kyot...@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 here is 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's no need to copy the partial debug_query_string into palloc'ed memory. Just replacing the query with query_loc will be sufficient. Have a nice holidays. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
