On Fri, Mar 19, 2021 at 09:29:06AM -0400, Bruce Momjian wrote:
> On Fri, Mar 19, 2021 at 11:16:50AM +0800, Julien Rouhaud wrote:
> > Now that I'm back on the code I remember why I did it this way. It's
> > unfortunately not really possible to make things work this way.
> >
> > pg_stat_statements' post_parse_analyze_hook relies on a queryid already being
> > computed, as it's where we know where the constants are recorded. It means:
> >
> > - we have to call post_parse_analyze_hook *after* doing core queryid
> > calculation
> > - if users want to use a third party module to calculate a queryid, they'll
> > have to make sure that the module's post_parse_analyze_hook is called
> > *before* pg_stat_statements' one.
> > - even if they do so, they'll still have to pay the price of core queryid
> > calculation
>
> OK, that makes perfect sense. I think the best solution is to document
> that compute_query_id just controls the built-in computation of the
> query id, and that extensions can also compute it if this is off, and
> pg_stat_activity and log_line_prefix will display built-in or extension
> computed query ids.
So the last version of the patch should implement that behavior right? It's
just missing some explicit guidance that third-party extensions should only
calculate a queryid if compute_query_id is off
> It might be interesting someday to check if the hook changed a
> pre-computed query id and warn the user in the logs, but that could
> cause more log-spam problems than help. I am a little worried that
> someone might have compute_query_id enabled and then install an
> extension that overwrites it, but we will just have to document this
> issue. Hopefully extensions will be clear that they are computing their
> own query id.
I agree. And hopefully they will split the queryid calculation from the rest
of the extension so that users can use the combination they want.