Thread

Re: Improve GetConfigOptionValues function

Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2023-01-19T09:56:34Z

> 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