Re: Allow pg_read_all_stats to read pg_stat_progress_*

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: x4mmm(at)yandex-team(dot)ru
Cc: magnus(at)hagander(dot)net, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Allow pg_read_all_stats to read pg_stat_progress_*
Date: 2020-04-16 05:05:04
Message-ID: 20200416.140504.900561658982478410.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Wed, 15 Apr 2020 15:58:05 +0500, "Andrey M. Borodin" <x4mmm(at)yandex-team(dot)ru> wrote in
> > 15 апр. 2020 г., в 15:25, Magnus Hagander <magnus(at)hagander(dot)net> написал(а):
> > I think that makes perfect sense. The documentation explicitly says "can read all pg_stat_* views", which is clearly wrong -- so either the code or the docs should be fixed, and it looks like it's the code that should be fixed to me.
> Should it be bug or v14 feature?
>
> Also pgstatfuncs.c contains a lot more checks of has_privs_of_role(GetUserId(), beentry->st_userid).
> Maybe grant them all?
>
> > As for the patch, one could argue that we should just store the resulting boolean instead of re-running the check (e.g. have a "bool has_stats_privilege" or such), but perhaps that's an unnecessary micro-optimization, like the attached.
>
> Looks good to me.

pg_stat_get_activty checks (has_privs_of_role() ||
is_member_of_role()) in-place for every entry. It's not necessary but
I suppose that doing the same thing for pg_stat_progress_info might be
better.

It's another issue, but pg_stat_get_backend_* functions don't consider
pg_read_all_stats. I suppose that the functions should work under the
same criteria to pg_stat views, and maybe explicitly documented?

If we do that, it may be better that we define "PGSTAT_VIEW_PRIV()" or
something like and replace the all occurances of the idiomatic
condition with it.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2020-04-16 05:18:28 Re: Include sequence relation support in logical replication
Previous Message Kyotaro Horiguchi 2020-04-16 03:08:09 Re: pg_restore: could not close data file: Success