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

On 2021-12-22 02:23, Tom Lane wrote:
> Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> writes:
>> At Mon, 20 Dec 2021 21:05:23 +0900, Shinya Kato
>> <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com> wrote in
>>> We should use EmitWarningsOnPlaceholders when we use
>>> DefineCustomXXXVariable.
>>> I don't think there is any room for debate.
>
>> Unfortunately, pltcl.c defines variables both in pltcl and pltclu but
>> the patch adds the warning only for pltclu.
>
> Right. The rest of 0001 looks fine, so pushed with that correction.
>
> I concur that 0002 is unacceptable. The result of it will be that
> a freshly-loaded extension will fail to hook into the system as it
> should, because it will fail to complete its initialization.
> That will cause user frustration and bug reports.
>
> (BTW, I wonder if EmitWarningsOnPlaceholders should be using LOG
> rather than WARNING when it's running in the postmaster. Use of
> WARNING is moderately likely to result in nothing getting printed
> at all.)
>
> 0003 looks to me like it was superseded by 75d22069e. I do not
> particularly like that patch though; it seems both inefficient
> and ill-designed. 0003 is adding a check in an equally bizarre
> place. Why isn't add_placeholder_variable the place to deal
> with this? Also, once we know that a prefix is reserved,
> why don't we throw an error not just a warning for attempts to
> set some unknown variable under that prefix? We don't allow
> you to set unknown non-prefixed variables.
>
> regards, tom lane

Thank you for pushing!
Thank you all for the reviews, I think I will take down 0002 and 0003.

--
Regards,

--
Shinya Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION