Re: Allow auto_explain to log to NOTICE

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: Tom Dunstan <tom(at)tomd(dot)cc>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, jdealmeidapereira(at)pivotal(dot)io
Subject: Re: Allow auto_explain to log to NOTICE
Date: 2018-07-17 18:03:40
Message-ID: 8A84058C-9CB6-4D4F-8DFC-9BD9E5795D9E@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 17 Jul 2018, at 19:11, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
>
> On 07/17/2018 12:04 PM, Daniel Gustafsson wrote:

>> Since DEBUG is not a defined loglevel, it seems superfluous to include it here.
>> It’s also omitted from the documentation so it should probably be omitted from
>> here.
>>
>> + {"debug", DEBUG2, true},
>
> I treated this like we do for client_min_messages and log_min_messages - the alias is there but I don;t think we document it either.
>
> I don't mind removing it, was just trying to be consistent. It seems odd that we would accept it in one place but not another.

Ooh.. I didn’t know that alias existed and didn’t find it when poking at the
code. In that case I agree with you, the alias should stay so I withdraw that
comment. I learned something new today =)

>>> I haven't added tests for auto_explain - I think that would be useful
>>> but it's a separate project.
>> Agreeing that this would be beneficial, the attached patch (to apply on top of
>> the patch in question) takes a stab at adding tests for this new functionality.
>>
>> In order to test plan output we need to support COSTS in the explain output, so
>> a new GUC auto_explain.log_costs is added. We also need to not print the
>> duration, so as a hack this patch omits the duration if auto_explain.log_timing
>> is set to off and auto_explain.log_analyze is set off. This is a hack and not
>> a nice overloading, but it seems overkill to add a separate GUC just to turn
>> off the duration, any better ideas on how support omitting the duration?
>
> Great, I'll check it out.

I’m not sure it’s worth adding this much to the code just to be able to test
it, but it seemed like a good excercise to write to have something to reason
about.

cheers ./daniel

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2018-07-17 18:10:32 Re: [HACKERS] logical decoding of two-phase transactions
Previous Message Alvaro Herrera 2018-07-17 18:01:25 Re: Internal error XX000 with enable_partition_pruning=on, pg 11 beta1 on Debian