Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?

On Sat, Oct 17, 2020 at 12:23 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> > In this case, I suppose using pg_stat_statement would require to have it
> > enabled, and it'd just not collect anything if disabled.

Yes, my idea was to be able to have pg_stat_statements enabled even if
no queryid is computed without that being a problem, and the patch I
sent should handle that properly, as pgss_store (and a few other
places) check for a non-zero queryid before doing any work.

Also, we can't have pg_stat_statements have any specific behavior
based on the new GUC, as there could alternatively be another module
that handles the queryid generation.

> Alternatively, pg_stat_statement might be able to force it on
> (applying a non-overridable PGC_INTERNAL-level setting) on load?
> Not sure if that'd be desirable or not.
>
> If the behavior of pg_stat_statement is to do nothing when it
> sees a query without the ID calculated (which I guess it'd have to)

Yes that's what it does.

> then there's a potential security issue if the GUC is USERSET level:
> a user could hide her queries from pg_stat_statement by turning the
> GUC off. So this line of thought suggests the GUC needs to be at
> least SUSET, and maybe higher ... doesn't pg_stat_statement need it
> to have the same value cluster-wide?

Well, I don't think that there's any guarantee that pg_stat_statemens
will display all activity that has been run, since there's a limited
amount of (userid, dbid, queryid) that can be stored, but I agree that
allowing random user to hide their activity isn't nice. Note that I
defined the GUC as SUSET, but maybe it should be SIGHUP?