Re: Improve GetConfigOptionValues function

On Wed, Jan 18, 2023 at 1:24 PM Nitin Jadhav
<nitinjadhavpostgres(at)gmail(dot)com> wrote:
>
> Attaching the patch.
>
> On Wed, Jan 18, 2023 at 1:21 PM Nitin Jadhav
> <nitinjadhavpostgres(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > 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.

Yes, the existing caller isn't using the fetched values when noshow is
set to true. However, I think returning from GetConfigOptionValues()
when noshow is set to true without fetching values limits the use of
the function. What if someother caller wants to use the function to
get the values with noshow passed in and use the values when noshow is
set to true?

Also, do we gain anything with the patch? I mean, can
show_all_settings()/pg_settings/pg_show_all_settings() get faster, say
with a non-superuser without pg_read_all_settings predefined role or
with a superuser? I see there're about 6 GUC_NO_SHOW_ALL GUCs and 20
GUC_SUPERUSER_ONLY GUCs, I'm not sure if it leads to some benefit with
the patch.

Having said above, I don't mind keeping the things the way they're right now.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com