Thread

Re: Improve GetConfigOptionValues function

Nitin Jadhav <nitinjadhavpostgres@gmail.com> — 2023-01-25T12:16:56Z

>>> 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@sss.pgh.pa.us> wrote:
>
> Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
> > On Mon, Jan 23, 2023 at 9:51 PM Tom Lane <tgl@sss.pgh.pa.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