Re: explain plans with information about (modified) gucs

On Tue, 26 Mar 2019 at 21:04, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:

> On Mon, Mar 18, 2019 at 11:31:48AM +0100, Rafia Sabih wrote:
> >On Sun, 24 Feb 2019 at 00:06, Tomas Vondra <tomas.vondra@2ndquadrant.com>
> wrote:
> >>
> >> Hi,
> >>
> >> attached is an updated patch, fixing and slightly tweaking the docs.
> >>
> >>
> >> Barring objections, I'll get this committed later next week.
> >>
> >I was having a look at this patch, and this kept me wondering,
> >
> >+static void
> >+ExplainShowSettings(ExplainState *es)
> >+{
> >Is there some reason for not providing any explanation above this
> >function just like the rest of the functions in this file?
> >
> >Similarly, for
> >
> >struct config_generic **
> >get_explain_guc_options(int *num)
> >{
> >
> >/* also bail out of there are no options */
> >+ if (!num)
> >+ return;
> >I think you meant 'if' instead if 'of' here.
>
> Thanks for the review - attached is a patch adding the missing comments,
> and doing two additional minor improvements:
>
> 1) Renaming ExplainShowSettings to ExplainPrintSettings, to make it more
> consistent with naming of the other functions in explain.c.
>
> 2) Actually counting GUC_EXPLAIN options, and only allocating space for
> those in get_explain_guc_options, instead of using num_guc_variables. The
> diffrence is quite significant (~50 vs. ~300), and considering each entry
> is 8B it makes a difference because such large chunks tend to have higher
> palloc overhed (due to ALLOCSET_SEPARATE_THRESHOLD).
>
>
> Looks like the patch is in need of a rebase.
At commit: 1983af8e899389187026cb34c1ca9d89ea986120

P.S. reject files attached.


-- 
Regards,
Rafia Sabih