Re: Allow pg_read_all_stats to read pg_stat_progress_*

Поиск
Список
Период
Сортировка
От Magnus Hagander
Тема Re: Allow pg_read_all_stats to read pg_stat_progress_*
Дата
Msg-id CABUevEzF+JY3CYUyMT4Tgh13MdhrHoduO-T4MfGvq6wqqCF7BA@mail.gmail.com
обсуждение исходный текст
Ответ на Allow pg_read_all_stats to read pg_stat_progress_*  ("Andrey M. Borodin" <x4mmm@yandex-team.ru>)
Ответы Re: Allow pg_read_all_stats to read pg_stat_progress_*  ("Andrey M. Borodin" <x4mmm@yandex-team.ru>)
Список pgsql-hackers


On Wed, Apr 15, 2020 at 9:14 AM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
Hi!

One of our users asked me why they cannot read details of pg_stat_progress_vacuum while they have pg_read_all_stats role.
Maybe I'm missing something, but I think they should be able to read stats...

PFA fix.
This affects pg_stat_progress_analyze, pg_stat_progress_basebackup, pg_stat_progress_cluster, pg_stat_progress_create_index and pg_stat_progress_vacuum.

With patch
postgres=# set role pg_read_all_stats ;
postgres=> select * from pg_stat_progress_vacuum ;
  pid  | datid | datname  | relid |     phase     | heap_blks_total | heap_blks_scanned | heap_blks_vacuumed | index_vacuum_count | max_dead_tuples | num_dead_tuples
-------+-------+----------+-------+---------------+-----------------+-------------------+--------------------+--------------------+-----------------+-----------------
 76331 | 12923 | postgres |  1247 | scanning heap |              10 |                 1 |                  0 |                  0 |            2910 |               0
(1 row)

Without patch
postgres=# set role pg_read_all_stats  ;
SET
postgres=> select * from pg_stat_progress_vacuum ;
  pid  | datid | datname  | relid | phase | heap_blks_total | heap_blks_scanned | heap_blks_vacuumed | index_vacuum_count | max_dead_tuples | num_dead_tuples
-------+-------+----------+-------+-------+-----------------+-------------------+--------------------+--------------------+-----------------+-----------------
 76331 | 12923 | postgres |       |       |                 |                   |                    |                    |                 |               
(1 row)

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.

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.

--
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: "Zhang, Jie"
Дата:
Сообщение: [PATHC] Fix minor memory leak in pg_basebackup
Следующее
От: "Andrey M. Borodin"
Дата:
Сообщение: Re: Allow pg_read_all_stats to read pg_stat_progress_*