Re: compute_query_id and pg_stat_statements

On Wed, May 12, 2021 at 09:51:26AM +0200, Pavel Stehule wrote:
>
> If I understand well, then computed_query_id does not make sense for
> pg_stat_statemenst, because this extension always requires it.

No, pg_stat_statements requires *a* queryid, not specifially *our* queryid.

> Cannot be better to use queryid inside pg_stat_statements every time
> without dependency on computed_query_id? And computed_query_id can be used
> only for EXPLAIN and for pg_stat_activity.

No, because then you will have a discrepancy between those two.  And if you
want a different queryid approach (say based on object names rather than oid so
it survives logical replication), then you also want that queryid used for
pg_stat_statements.  And that what happen is that you have to fork
pg_stat_statements to only change the queryid implementation, which is one of
the thing that the patch to move the implementation to core solves.

> pg_stat_statements cannot work without a queryid, so is useless to speak
> about configuration. If you use pg_stat_statements, then the queryid will
> be computed every time, but the visibility will be only for
> pg_stat_statements.

Yes, pg_stat_statements cannot work without a queryid, but it CAN work without
core queryid.

> Or a different strategy. I understand so computed_query_id should be
> active. But I dislike the empty result of pg_stat_statements when
> computed_query_id is off. Is it possible to raise an exception instead of
> showing an empty result?

Yes, but I don't think that it's a good idea.  For instance pg_stat_statements
will behave poorly if you have to regularly evict entry.  For instance: any
query touching a temporary table.  One way to avoid that it to avoid storing
entries that you know are very likely to be eventually evicted.

So to fix this problem, you have 2 ways to go:

1) fix your app and explicitly disable/enable pg_stat_statements around all
  those queries, and hope you won't miss any

2) write your own queryid implementation to not generate a queryid in such case.

2 seems like a reasonable scenario, and if you force pg_stat_statements to
error out in that case then you would be forced to use approach 1.

My second proposal can work for your example too. pg_stat_statements have to require any active queryid computing. And when it is not available, then the exception should be raised.

The custom queryid can return null, and still the queryid will be computed. Maybe the warning can be enough. Just, if somebody use pg_stat_statements function, then enforce the check if queryid is computed (compute_query_id is true || some hook is not null), and if not then raise a warning.