From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: peter.eisentraut@enterprisedb.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: Wed, 22 Dec 2021 16:50:34 +0900
Message-ID: <f055f52eeedba4a4fc69a3e0dc5dc176@oss.nttdata.com> (raw)
In-Reply-To: <57009.1640107405@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>
On 2021-12-22 02:23, Tom Lane wrote:
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
>> At Mon, 20 Dec 2021 21:05:23 +0900, Shinya Kato
>> <Shinya11.Kato@oss.nttdata.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
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, horikyota.ntt@gmail.com, peter.eisentraut@enterprisedb.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: <f055f52eeedba4a4fc69a3e0dc5dc176@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