From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: peter.eisentraut@enterprisedb.com
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: masao.fujii@oss.nttdata.com
Cc: bharath.rupireddyforpostgres@gmail.com
Cc: daniel@yesql.se
Cc: pgsql-hackers@postgresql.org
Subject: Re: Emit a warning if the extension's GUC is set incorrectly
Date: Fri, 24 Dec 2021 12:29:47 +0900
Message-ID: <65bac0ad-0a5c-c841-8812-04285871549f@oss.nttdata.com> (raw)
In-Reply-To: <116024.1640111629@sss.pgh.pa.us>
References: <9f5e96ca44891915f1d2736bea9fdf45@oss.nttdata.com>
<83b33009-3a1b-9230-8f07-2115763c1853@enterprisedb.com>
<c5660e836dd511549810fd35b12edc3c@oss.nttdata.com>
<20211221.100834.1297468238807011575.horikyota.ntt@gmail.com>
<57009.1640107405@sss.pgh.pa.us>
<116024.1640111629@sss.pgh.pa.us>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: Shinya11.Kato@oss.nttdata.com, tgl@sss.pgh.pa.us, peter.eisentraut@enterprisedb.com, horikyota.ntt@gmail.com, masao.fujii@oss.nttdata.com, bharath.rupireddyforpostgres@gmail.com, daniel@yesql.se
Subject: Re: Emit a warning if the extension's GUC is set incorrectly
In-Reply-To: <65bac0ad-0a5c-c841-8812-04285871549f@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox