Kyotaro Horiguchi <horikyota....@gmail.com> writes: > At Mon, 20 Dec 2021 21:05:23 +0900, Shinya Kato > <shinya11.k...@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