At Wed, 15 Apr 2020 15:58:05 +0500, "Andrey M. Borodin" <x4mmm@yandex-team.ru> wrote in > > 15 апр. 2020 г., в 15:25, Magnus Hagander <magnus@hagander.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.
From a result perspective, it shouldn't make a difference though, should it? It's a micro-optimization, but it might not have an actual performance effect in reality as well, but the result should always be the same?
(FWIW, pg_stat_statements has a coding pattern similar to the one I suggested in the patch)
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?
That's a good question. They haven't been documented to do so, but it certainly seems *weird* that the same information should be available through a view like pg_stat_activity, but not through the functions.
I would guess this was simply forgotten in 25fff40798f -- I don't recall any discussion about it. The commit message specifically says pg_database_size() and pg_tablespace_size(), but mentions nothing about pg_stat_*.
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.
