Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation) - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation)
Date
Msg-id 12027.1332868550@sss.pgh.pa.us
Whole thread Raw
In response to Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation)  (Peter Geoghegan <peter@2ndquadrant.com>)
Responses Re: Re: pg_stat_statements normalisation without invasive changes to the parser (was: Next steps on pg_stat_statements normalisation)
List pgsql-hackers
Peter Geoghegan <peter@2ndquadrant.com> writes:
>> On 22 March 2012 17:19, Tom Lane <tgl@sss.pgh.pa.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


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Command Triggers patch v18
Next
From: Peter Eisentraut
Date:
Subject: Re: Another review of URI for libpq, v7 submission