Re: Improve GetConfigOptionValues function

From: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improve GetConfigOptionValues function
Date: 2023-01-25 12:16:56
Message-ID: CAMm1aWa4TFoZbNh+aF36S0Xg7ggrvugh-h9vpMweW4e3ui3bLA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>> Also, I intentionally dropped the GUC_NO_SHOW_ALL check in
>>> get_explain_guc_options, because it seems redundant given
>>> the preceding GUC_EXPLAIN check. It's unlikely we'd ever have
>>> a variable that's marked both GUC_EXPLAIN and GUC_NO_SHOW_ALL ...
>>> but if we did, shouldn't the former take precedence here anyway?
>
>> You're right, but there's nothing that prevents users writing GUCs
>> with GUC_EXPLAIN and GUC_NO_SHOW_ALL.
>
> "Users"? You do realize those flags are only settable by C code,
> right? Moreover, you haven't explained why it would be good that
> you can't get at the behavior that a GUC is both shown in EXPLAIN
> and not shown in SHOW ALL. If you want "not shown by either",
> that's already accessible by setting only the GUC_NO_SHOW_ALL
> flag. So I'd almost argue this is a bug fix, though I concede
> it's a bit hard to imagine why somebody would want that choice.
> Still, if we have two independent flags they should produce four
> behaviors, not just three.

I agree that the developer can use both GUC_NO_SHOW_ALL and
GUC_EXPLAIN knowingly or unknowingly for a single GUC. If used by
mistake then according to the existing code (without patch),
GUC_NO_SHOW_ALL takes higher precedence whether it is marked first or
last in the code. I am more convinced with this behaviour as I feel it
is safer than exposing the information which the developer might not
have intended.

Thanks & Regards,
Nitin Jadhav

On Tue, Jan 24, 2023 at 8:43 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> writes:
> > On Mon, Jan 23, 2023 at 9:51 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> Also, I intentionally dropped the GUC_NO_SHOW_ALL check in
> >> get_explain_guc_options, because it seems redundant given
> >> the preceding GUC_EXPLAIN check. It's unlikely we'd ever have
> >> a variable that's marked both GUC_EXPLAIN and GUC_NO_SHOW_ALL ...
> >> but if we did, shouldn't the former take precedence here anyway?
>
> > You're right, but there's nothing that prevents users writing GUCs
> > with GUC_EXPLAIN and GUC_NO_SHOW_ALL.
>
> "Users"? You do realize those flags are only settable by C code,
> right? Moreover, you haven't explained why it would be good that
> you can't get at the behavior that a GUC is both shown in EXPLAIN
> and not shown in SHOW ALL. If you want "not shown by either",
> that's already accessible by setting only the GUC_NO_SHOW_ALL
> flag. So I'd almost argue this is a bug fix, though I concede
> it's a bit hard to imagine why somebody would want that choice.
> Still, if we have two independent flags they should produce four
> behaviors, not just three.
>
> regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Devrim Gündüz 2023-01-25 13:00:03 Re: drop postmaster symlink
Previous Message Nitin Jadhav 2023-01-25 12:06:22 Re: Improve GetConfigOptionValues function