Re: RFC: Logging plan of the running query

From: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: RFC: Logging plan of the running query
Date: 2021-05-28 06:51:37
Message-ID: c6682a25f3f0e9bd520707342219eac5@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-05-13 21:57, Dilip Kumar wrote:
> On Thu, May 13, 2021 at 5:18 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com>
> wrote:
>>
>> On Thu, May 13, 2021 at 5:15 PM Bharath Rupireddy
>> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>> >
>> > On Thu, May 13, 2021 at 5:14 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> > >
>> > > On Thu, May 13, 2021 at 4:16 PM Bharath Rupireddy
>> > > <bharath(dot)rupireddyforpostgres(at)gmail(dot)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.

> Bharat offlist pointed to me that here he was talking about another
> log that is logging the query and not specific to this patch, so let's
> not discuss this point here.

Thanks for sharing the situation!

--
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION

Attachment Content-Type Size
v2-0001-log-running-query-plan.patch text/x-diff 11.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message tanghy.fnst@fujitsu.com 2021-05-28 07:01:06 RE: [BUG]Update Toast data failure in logical replication
Previous Message Kyotaro Horiguchi 2021-05-28 06:25:40 Re: Bracket, brace, parenthesis