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

On 2021-12-17 01:55, Fujii Masao wrote:
> 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 these are the valid
> steps, but with the patch, the server fails to start up at the step #2
> because pg_trgm's custom parameters are 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

It can happen because the placeholder "pg_trgm" is not already
registered.
I think too this behavior is not good.
I need to consider an another implementation or to allow undefined GUCs
to be set.

> 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,

Thanks! There was a correction omission, so I fixed it.

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

Since I get an ERROR when I set the wrong normal GUCs, I thought I
should also get an ERROR when I set the wrong extension's GUCs.
However, there are likely to be harmful effects, and I may need to think
again.

For now, I'v attached the patch that fixed the compilation error.

--
Regards,

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