Re: RFC: Logging plan of the running query

On 2021-05-28 15:51, torikoshia wrote:
> On 2021-05-13 21:57, Dilip Kumar wrote:
>> On Thu, May 13, 2021 at 5:18 PM Dilip Kumar <dilipbalaut@gmail.com> 
>> wrote:
>>> 
>>> On Thu, May 13, 2021 at 5:15 PM Bharath Rupireddy
>>> <bharath.rupireddyforpostgres@gmail.com> wrote:
>>> >
>>> > On Thu, May 13, 2021 at 5:14 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>> > >
>>> > > On Thu, May 13, 2021 at 4:16 PM Bharath Rupireddy
>>> > > <bharath.rupireddyforpostgres@gmail.com> wrote:
>>> > > >
>>> > > > I'm saying that -  currently, queries are logged with LOG level when
>>> > > > the log_statement GUC is set. The queries might be sent to the
>>> > > > non-superuser clients. So, your point of "sending the plan to those
>>> > > > clients is not a good idea from a security perspective" gets violated
>>> > > > right? Should the log level be changed(in the below code) from "LOG"
>>> > > > to "LOG_SERVER_ONLY"? I think we can discuss this separately so as not
>>> > > > to sidetrack the main feature.
>>> > > >
>>> > > >     /* Log immediately if dictated by log_statement */
>>> > > >     if (check_log_statement(parsetree_list))
>>> > > >     {
>>> > > >         ereport(LOG,
>>> > > >                 (errmsg("statement: %s", query_string),
>>> > > >                  errhidestmt(true),
>>> > > >                  errdetail_execute(parsetree_list)));
>>> > > >
>>> > >
>>> > > Yes, that was my exact point, that in this particular code log with
>>> > > LOG_SERVER_ONLY.
>>> > >
>>> > > Like this.
>>> > >      /* Log immediately if dictated by log_statement */
>>> > >      if (check_log_statement(parsetree_list))
>>> > >      {
>>> > >          ereport(LOG_SERVER_ONLY,
>>> >
>>> > Agree, but let's discuss that in a separate thread.
>>> 
>>> Did not understand why separate thread? this is part of this thread
>>> no? but anyways now everyone agreed that we will log with
>>> LOG_SERVER_ONLY.
> 
> Modified elevel from LOG to LOG_SERVER_ONLY.
> 
> I also modified the patch to log JIT Summary and GUC settings 
> information.
> If there is other useful information to log, I would appreciate it if
> you could point it out.

Updated the patch.

- reordered superuser check which was pointed out in another thread[1]
- added a regression test

[1] https://postgr.es/m/YLxw1uVGIAP5uMPl@paquier.xyz

Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION