Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl

From: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl
Date: 2023-02-04 19:26:58
Message-ID: CAMm1aWZ9g9eEMmRFu124O5h=HQsDFM5mdxiPND_hFKHp1d-69g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> I don't particularly see why that needs to be the case. Notably,
> if we're interested in enforcing a policy even for extension GUCs,
> guc.sql can't really do that since who knows whether the extension's
> author will bother to run that test with the extension loaded.
> On the other hand, moving *all* those checks into guc.c is probably
> impractical and certainly will add undesirable startup overhead.

Ok. Understood the other problems. I have attached the v2 patch which
uses the idea present in Michael's patch. In addition, I have removed
fetching NO_SHOW_ALL parameters while creating tab_settings_flags
table in guc.sql and adjusted the test which checks for (NO_RESET_ALL
AND NOT NO_SHOW_ALL) as this was misleading the developer who thinks
that tab_settings_flags table has NO_SHOW_ALL parameters which is
incorrect.

Please review and share your thoughts.

Thanks & Regards,
Nitin Jadhav

On Sat, Feb 4, 2023 at 1:07 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com> writes:
> > My concern is if we do this, then we will end up having some policies
> > (which can be read from pg_show_all_settings()) in guc.sql and some in
> > guc.c. I feel all these should be at one place either at guc.c or
> > guc.sql.
>
> I don't particularly see why that needs to be the case. Notably,
> if we're interested in enforcing a policy even for extension GUCs,
> guc.sql can't really do that since who knows whether the extension's
> author will bother to run that test with the extension loaded.
> On the other hand, moving *all* those checks into guc.c is probably
> impractical and certainly will add undesirable startup overhead.
>
> regards, tom lane

Attachment Content-Type Size
v2-0001-Fix-GUC_NO_SHOW_ALL-test-scenarios.patch application/octet-stream 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2023-02-04 20:32:45 Re: proposal: psql: show current user in prompt
Previous Message Peter Geoghegan 2023-02-04 19:10:55 Re: BUG: Postgres 14 + vacuum_defer_cleanup_age + FOR UPDATE + UPDATE