Re: Improve GetConfigOptionValues function - Mailing list pgsql-hackers

From Nitin Jadhav
Subject Re: Improve GetConfigOptionValues function
Date
Msg-id CAMm1aWZU2wpMODWc==oPKM+AZdMBuCJtCprki-gvb7fSfs8Png@mail.gmail.com
Whole thread Raw
In response to Re: Improve GetConfigOptionValues function  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Improve GetConfigOptionValues function
List pgsql-hackers
> Possibly a better answer is to refactor into separate functions,
> along the lines of
>
> static bool
> ConfigOptionIsShowable(struct config_generic *conf)
>
> static void
> GetConfigOptionValues(struct config_generic *conf, const char **values)

Nice suggestion. Attached a patch for the same. Please share the
comments if any.

Thanks & Regards,
Nitin Jadhav

On Wed, Jan 18, 2023 at 9:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Nitin Jadhav <nitinjadhavpostgres@gmail.com> writes:
> > GetConfigOptionValues function extracts the config parameters for the
> > given variable irrespective of whether it results in noshow or not.
> > But the parent function show_all_settings ignores the values parameter
> > if it results in noshow. It's unnecessary to fetch all the values
> > during noshow. So a return statement in GetConfigOptionValues() when
> > noshow is set to true is needed. Attached the patch for the same.
> > Please share your thoughts.
>
> I do not think this is an improvement: it causes GetConfigOptionValues
> to be making assumptions about how its results will be used.  If
> show_all_settings() were a big performance bottleneck, and there were
> a lot of no-show values that we could optimize, then maybe the extra
> coupling would be worthwhile.  But I don't believe either of those
> things.
>
> Possibly a better answer is to refactor into separate functions,
> along the lines of
>
> static bool
> ConfigOptionIsShowable(struct config_generic *conf)
>
> static void
> GetConfigOptionValues(struct config_generic *conf, const char **values)
>
>                         regards, tom lane

Attachment

pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Use appendStringInfoSpaces more
Next
From: Julien Rouhaud
Date:
Subject: Re: Record queryid when auto_explain.log_verbose is on