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

On 2021/12/16 16:31, Shinya Kato wrote:
> 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.

IMO this behavior change is not good. For example, because it seems to break at least the following case. I think that
theseare the valid steps, but with the patch, the server fails to start up at the step #2 because pg_trgm's custom
parametersare treated as invalid ones.
 

1. Add the following two pg_trgm parameters to postgresql.conf
     - pg_trgm.similarity_threshold
     - pg_trgm.strict_word_similarity_threshold

2. Start the server

3. Run "CREATE EXTENSION pg_trgm" and then use pg_trgm


When I compiled PostgreSQL with the patch, I got the following
compilation failure.

guc.c:5453:4: error: implicit declaration of function 'EmitErrorsOnPlaceholders' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
                         EmitErrorsOnPlaceholders(placeholder);
                         ^


-            ereport(WARNING,
+            ereport(ERROR,

I'm still not sure why you were thinking that ERROR is more proper here.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION