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

On 2021/12/22 3:33, Tom Lane wrote:
> I wrote:
>> 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.
> Concretely, I think we should do the attached, which removes much of
> 75d22069e and does the check at the point of placeholder creation.
> (After looking closer, add_placeholder_variable's caller is the
> function that is responsible for rejecting bad names, not
> add_placeholder_variable itself.)
>
> This also fixes an indisputable bug in 75d22069e, namely that it
> did prefix comparisons incorrectly and would throw warnings for
> cases it shouldn't.  Reserving "plpgsql" shouldn't have the effect
> of reserving "pl" as well.

Thank you for creating the patch.
This is exactly what I wanted to create. It looks good to me.


> I'm tempted to propose that we also rename EmitWarningsOnPlaceholders
> to something like MarkGUCPrefixReserved, to more clearly reflect
> what it does now.  (We could provide the old name as a macro alias
> to avoid breaking extensions needlessly.)

+1


-- 
Regards,

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