> On 17 Jul 2018, at 19:11, Andrew Dunstan <andrew.duns...@2ndquadrant.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