On Fri, Oct 22, 2021 at 3:15 AM Bossart, Nathan <bossartn@amazon.com> wrote:
>
> On 10/20/21, 11:44 PM, "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com> wrote:
> > I would like to confine this thread to allowing non-superusers with a
> > predefined role (earlier suggestion was to use pg_read_all_stats) to
> > access views pg_backend_memory_contexts and pg_shmem_allocations and
> > functions pg_get_backend_memory_contexts and pg_get_shmem_allocations.
> > Attaching the previous v2 patch here for further review and thoughts.
>
> I took a look at the new patch. The changes to system_views.sql look
> good to me.
Thanks for reviewing.
> Let's be sure to update doc/src/sgml/catalogs.sgml as
> well.
Added.
> -SELECT * FROM pg_log_backend_memory_contexts(pg_backend_pid());
> +SELECT pg_log_backend_memory_contexts(pg_backend_pid());
>
> nitpick: Do we need to remove the "* FROM" here? This seems like an
> unrelated change.
Yes it's not mandatory, while we are on this I thought we could
combine them, I've also specified this in the commit message. IMO, we
can leave it to the committer.
> +-- test to check privileges of system views pg_shmem_allocations,
> +-- pg_backend_memory_contexts and function pg_log_backend_memory_contexts.
>
> I think the comment needs to be updated to remove the reference to
> pg_log_backend_memory_contexts. It doesn't appear to be tested here.
Removed.
> +SELECT name, ident, parent, level, total_bytes >= free_bytes
> + FROM pg_backend_memory_contexts WHERE level = 0; -- permission denied error
> +SELECT COUNT(*) >= 0 AS ok FROM pg_shmem_allocations; -- permission denied error
>
> Since we're really just checking the basic permissions, could we just
> do the "count(*) >= 0" check for both views?
Done.
Here's v3 for further review.
Regards,
Bharath Rupireddy.
