Re: Emit a warning if the extension's GUC is set incorrectly

Florin Irion <irionr@gmail.com>

From: Florin Irion <irionr@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Shinya11.Kato@oss.nttdata.com, masao.fujii@oss.nttdata.com, bharath.rupireddyforpostgres@gmail.com, daniel@yesql.se, pgsql-hackers@postgresql.org
Date: 2022-02-18T11:47:23Z
Lists: pgsql-hackers
Il giorno ven 18 feb 2022 alle ore 10:58 Tom Lane <tgl@sss.pgh.pa.us> ha
scritto:

> I wrote:
> > As a stopgap to turn the farm green again, I am going to revert
> > 75d22069e as well as my followup patches.  If we don't want to
> > give up on that idea altogether, we have to find some way to
> > suppress the chatter from parallel workers.  I wonder whether
> > it would be appropriate to go further than we have, and actively
> > delete placeholders that turn out to be within an extension's
> > reserved namespace.  The core issue here is that workers don't
> > necessarily set GUCs and load extensions in the same order that
> > their parent did, so if we leave any invalid placeholders behind
> > after reserving an extension's prefix, we're risking issues
> > during worker start.
>
> Here's a delta patch (meant to be applied after reverting cab5b9ab2)
> that does things like that.  It fixes the parallel-mode problem ...
> so do we want to tighten things up this much?
>
>                         regards, tom lan
>

Hello,

Thank you for taking care of the bug I introduced with 75d22069e,
I didn't notice this thread until now.

For what it's worth, I agree with throwing an ERROR if the placeholder is
unrecognized. Initially, I didn't want to change too much the liberty of
setting any
placeholder, but mainly to not go unnoticed.

In my humble opinion, I still think that this should go on and disallow
bogus placeholders as we do for postgres unrecognized settings.

I tested your delta patch with and without parallel mode, and, naturally,
it behaves correctly.

My 2 cents.

+1

Cheers,
Florin Irion

Commits

  1. Disallow setting bogus GUCs within an extension's reserved namespace.

  2. Revert changes about warnings/errors for placeholders.

  3. Rename EmitWarningsOnPlaceholders() to MarkGUCPrefixReserved().

  4. Rethink handling of settings with a prefix reserved by an extension.

  5. Add missing EmitWarningsOnPlaceholders() calls.

  6. Add EmitWarningsOnPlaceholders calls to contrib modules that are likely to