Re: BUG: pg_stat_statements query normalization issues with combined queries

At Thu, 26 Jan 2017 22:34:57 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <23778(dot)1485488097(at)sss(dot)pgh(dot)pa(dot)us>
> Craig Ringer <craig(dot)ringer(at)2ndquadrant(dot)com> writes:
> > So perhaps:
>
> > "The same query string may be passed to multiple invocations of
> > ProcessUtility if a utility statement invokes subcommands (e.g. ALTER
> > TABLE), in which case context will be set to
> > PROCESS_UTILITY_SUBCOMMAND, or if the user supplied a query string
> > containing multiple semicolon-separated statements in a single
> > protocol message. It is also possible for the query text to contain
> > other non-utility-statement text like comments, empty statements, and
> > plannable statements that don't pass through ProcessUtility. Hooks
> > that use the queryString should use pstmt->stmt_location and
> > pstmt->stmt_len to extract the text for the statement of interest and
> > should pay attention to the context to avoid repeatedly handling the
> > same query string in subcommands."
>
> Meh. I really don't think that a comment about the query string is
> the place to get into promises that are unrelated to the string, and
> that ProcessUtility is in no position to enforce anyway. How about
> we just say this:
>
> "The same queryString may be passed to multiple invocations of
> ProcessUtility when processing a query string containing multiple
> semicolon-separated statements; one should use pstmt->stmt_location and
> pstmt->stmt_len to identify the substring containing the current
> statement. Keep in mind also that some utility statements (e.g.,
> CREATE SCHEMA) will recurse to ProcessUtility to process sub-statements,
> often passing down the same queryString, stmt_location, and stmt_len
> that were given for the whole statement."

Tom's rewrite is easier to read for me and seems telling me
enough as a user of the hook.

By the way the existing comment for the hook,

> *
> * We provide a function hook variable that lets loadable plugins get
> * control when ProcessUtility is called. Such a plugin would normally
> * call standard_ProcessUtility().
> */

This is quite a matter of course for developers. planner_hook and
other ones don't have such a comment or have a one-liner at
most. Since this hook gets a good amount of commnet, it seems
better to just remove the existing one..

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center