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

Thank you for the review and sorry for the late reply.

On 2021-11-16 19:25, Bharath Rupireddy wrote:
>> > I observed an odd behaviour:
>> > 1) I set postgres_fdw.XXX = 'I_messed_up_conf_file' in postgresql.conf
>> > 2) With EmitWarningsOnPlaceholders("postgres_fdw"); in postgres_fdw
>> > contrib module, I created the extension, have seen the following
>> > warning:
>> > 2021-11-15 06:02:31.198 UTC [2018111] WARNING:  unrecognized
>> > configuration parameter "postgres_fdw.XXX"
>> > 3) I further did, "alter system set
>> > postgres_fdw.XXX='I_further_messed_up_conf_file';" and also "select
>> > pg_reload_conf();", it silently accepts.
>> >
>> > postgres=# create extension postgres_fdw ;
>> > WARNING:  unrecognized configuration parameter "postgres_fdw.XXX"
>> > CREATE EXTENSION
>> > postgres=# alter system set
>> > postgres_fdw.XXX='I_further_messed_up_conf_file';
>> > ALTER SYSTEM
>> > postgres=# select pg_reload_conf();
>> >  pg_reload_conf
>> > ----------------
>> >  t
>> > (1 row)

I have made changes to achieve the above.
However, it also gives me an error when
---
postgres=# SET abc.a TO on;
SET
postgres=# SET abc.b TO on;
2021-12-16 16:22:20.351 JST [2489236] ERROR:  unrecognized configuration 
parameter "abc.a"
2021-12-16 16:22:20.351 JST [2489236] STATEMENT:  SET abc.b TO on;
ERROR:  unrecognized configuration parameter "abc.a"
---

I know that some people do not think this is good.
Therefore, can I leave this problem alone or is there another better 
way?


> For backward compatibility you can retain the function
> EmitWarningsOnPlaceholders as-is and have another similar function
> that emits an error:
> 
> In guc.c, have something like below:
> static void
> check_conf_params(const char *className, bool emit_error)
> {
>    /* have the existing code of EmitWarningsOnPlaceholders here */
>             ereport(emit_error ? ERROR : WARNING,
>                     (errcode(ERRCODE_UNDEFINED_OBJECT),
>                      errmsg("unrecognized configuration parameter 
> \"%s\"",
>                             var->name)));
> }
> 
> void
> EmitErrorOnPlaceholders(const char *className)
> {
>     check_conf_params(className, true);
> }
> 
> void
> EmitWarningsOnPlaceholders(const char *className)
> {
>     check_conf_params(className, false);
> }
> 

Thank you for your advise.
According to [1], we used the same function name, but the warning level 
was INFO.
Therefore, I think it is OK to use the same function name.

[1] 
https://www.postgresql.org/message-id/flat/200901051634.n05GYNr06169%40momjian.us#1d045374f014494e4b40a4862a000723

-- 
Regards,

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