From: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
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-31 12:23:55 |
Message-ID: | 67101522-39d2-e302-92f1-b62b6d762a56@2ndQuadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 07/17/2018 02:03 PM, Daniel Gustafsson wrote:
>> 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 =)
Committed with the doc fix belatedly.
>>>> 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.
>
I think it probably is, buit I'm not very happy about the hack, so I
didn't commit it. Please submit this to the next commitfest, possibly
with a nicer way of managing the duration logging.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Fabien COELHO | 2018-07-31 12:41:55 | Re: Doc patch: add RECURSIVE to bookindex |
Previous Message | Robert Haas | 2018-07-31 11:41:58 | Re: Bizarre behavior in libpq's searching of ~/.pgpass |