From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Ekaterina Sokolova <e(dot)sokolova(at)postgrespro(dot)ru> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> |
Subject: | Re: RFC: Logging plan of the running query |
Date: | 2021-11-13 13:29:59 |
Message-ID: | CALj2ACWGR79n+R=VJXTFYpADYkxb=Fd8XVDdwhnBV1NsAGzzpg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 13, 2021 at 7:58 PM Ekaterina Sokolova
<e(dot)sokolova(at)postgrespro(dot)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(at)postgresql(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | Euler Taveira | 2021-11-13 13:46:07 | Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation |
Previous Message | Bharath Rupireddy | 2021-11-13 13:15:49 | Re: Allow users to choose what happens when recovery target is not reached |