Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation)

Peter Geoghegan <peter(at)2ndquadrant(dot)com> writes:
>> On 22 March 2012 17:19, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Either way, I think we'd be a lot better advised to define a single
>>> hook "post_parse_analysis_hook" and make the core code responsible
>>> for calling it at the appropriate places, rather than supposing that
>>> the contrib module knows exactly which core functions ought to be
>>> the places to do it.

> I have done this too.

The "canonicalize" argument to the proposed hook seems like a bit of a
crock. You've got the core code magically setting that in a way that
responds to extremely pg_stat_statements-specific concerns, and I am not
very sure it's right even for those concerns.

I am thinking that perhaps a reasonable signature for the hook function
would be

void post_parse_analyze (ParseState *pstate, Query *query);

with the expectation that it could dig whatever it wants to know out
of the ParseState (in particular the sourceText is available there,
and in general this should provide everything that's known at parse
time).

Now, if what it wants to know about is the parameterization status
of the query, things aren't ideal because most of the info is hidden
in parse-callback fields that aren't of globally exposed types. However
we could at least duplicate the behavior you have here, because you're
only passing canonicalize = true in cases where no parse callback will
be registered at all, so pg_stat_statements could equivalently test for
pstate->p_paramref_hook == NULL.

Thoughts, other ideas?

regards, tom lane