On Wed, Oct 13, 2021 at 7:58 PM Ekaterina Sokolova
<e.sokolova@postgrespro.ru> wrote:
> Thank you for working on this issue. I would be glad to continue to
> follow the development of this issue.
Thanks for the patch. I'm not sure if v11 is the latest patch, if yes,
I have the following comments:
1) Firstly, v11 patch isn't getting applied on the master -
http://cfbot.cputube.org/patch_35_3142.log.
2) I think we are moving away from if (!superuser()) checks, see the
commit [1]. The goal is to let the GRANT-REVOKE system deal with who
is supposed to run these system functions. Since
pg_log_current_query_plan also writes the info to server logs, I think
it should do the same thing as commit [1] did for
pg_log_backend_memory_contexts.
With v11, you are re-introducing the superuser() check in the
pg_log_backend_memory_contexts which is wrong.
3) I think SendProcSignalForLogInfo can be more generic, meaning, it
can also send signal to auxiliary processes if asked to do this will
simplify the things for pg_log_backend_memory_contexts and other
patches like pg_print_backtrace. I would imagine it to be "bool
SendProcSignalForLogInfo(pid_t pid, ProcSignalReason reason, bool
signal_aux_proc);".
[1] commit f0b051e322d530a340e62f2ae16d99acdbcb3d05
Author: Jeff Davis <jdavis@postgresql.org>
Date: Tue Oct 26 13:13:52 2021 -0700
Allow GRANT on pg_log_backend_memory_contexts().
Remove superuser check, allowing any user granted permissions on
pg_log_backend_memory_contexts() to log the memory contexts of any
backend.
Note that this could allow a privileged non-superuser to log the
memory contexts of a superuser backend, but as discussed, that does
not seem to be a problem.
Reviewed-by: Nathan Bossart, Bharath Rupireddy, Michael Paquier,
Kyotaro Horiguchi, Andres Freund
Discussion:
https://postgr.es/m/e5cf6684d17c8d1ef4904ae248605ccd6da03e72.camel@j-davis.com
Regards,
Bharath Rupireddy.