Re: Improve GetConfigOptionValues function

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improve GetConfigOptionValues function
Date: 2023-01-27 04:05:41
Message-ID: CALj2ACXihbqYSQE2sr0o-JBgT_cVH-H_+1b-YLpQhhx4X5QGpg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

I meant extensions here.

> 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.

Got it. I think I should've looked at GUC_NO_SHOW_ALL and GUC_EXPLAIN
as separate flags not depending on each other in any way, meaning,
GUCs marked with GUC_NO_SHOW_ALL mustn't be shown in SHOW ALL and its
friends
pg_settings and pg_show_all_settings(), and GUCs marked with
GUC_EXPLAIN must be shown in explain output. This understanding is
clear and simple IMO.

Having said that, I have no objection to the v4 patch posted upthread.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2023-01-27 04:11:53 RE: Exit walsender before confirming remote flush in logical replication
Previous Message Justin Pryzby 2023-01-27 03:56:32 Re: Add LZ4 compression in pg_dump