Re: compute_query_id and pg_stat_statements

On Thu, May 13, 2021 at 09:30:55AM -0700, Maciek Sakrejda wrote:
> On Thu, May 13, 2021 at 8:38 AM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> >
> > On Thu, May 13, 2021 at 08:32:50AM -0700, Maciek Sakrejda wrote:
> > >
> > > The warning makes it clear that there's something wrong, but not how
> > > to fix it
> >
> > I'm confused, are we talking about the new warning in v2 as suggested by Pavel?
> > As it should make things quite clear:
> >
> > +SELECT count(*) FROM pg_stat_statements;
> > +WARNING: Query identifier calculation seems to be disabled
> > +HINT: If you don't want to use a third-party module to compute query identifiers, you may want to enable compute_query_id
> >
> > The wording can of course be improved.
>
> I meant that no warning can be as clear as things just working, but I
> do have feedback on the specific message here:
>
> * "seems to" be disabled? Is it? Any reason not to be more definitive here?
> * On reading the beginning of the hint, I can see users asking
> themselves, "Do I want to use a third-party module to compute query
> identifiers?"
> * "may want to enable"? Are there any situations where I don't want
> to use a third-party module *and* I don't want to enable
> compute_query_id?
>
> So maybe something like
>
> > +SELECT count(*) FROM pg_stat_statements;
> > +WARNING: Query identifier calculation is disabled
> > +HINT: You must enable compute_query_id or configure a third-party module to compute query identifiers in order to use pg_stat_statements.

Yes, I like this. The reason the old message was so vague is that
'auto', the default some people wanted, didn't issue that error, only
'off' did, so there was an assumption you wanted a custom module since
you changed the value to off. If we are going with just on/off, no
auto, the message you suggest, leading with compute_query_id, is the
right approach.

> (I admit "configure a third-party module" is pretty vague, but I think
> that suggests it's only an option to consider if you know what you're
> doing.)

Seems fine to me.

> Also, if you're configuring this for usage with a tool like pganalyze,
> and neglect to run a manual query (we guide users to do that, but they
> may skip that step), the warnings may not even be visible (the Go
> driver we are using does not surface client warnings). Should this be
> an error instead of a warning? Is it ever useful to get an empty
> result set from querying pg_stat_statements? Using an error here would
> parallel the behavior of shared_preload_libraries not including
> pg_stat_statements.

Good question.

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

If only the physical world exists, free will is an illusion.