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

On 2021-11-15 15:14, Bharath Rupireddy wrote:

> Do we need to add them in the following too?
> 
> delay_execution/delay_execution.c
> ssl_passphrase_func.c
> worker_spi.c
> 
> Especially, worker_spi.c is important as it works as a template for
> the bg worker code.

Thank you for pointing this out! I added it.

> I'm not sure if we have "how to create an extension/a bg worker?" in
> the core docs, if we have, it is a good idea to make note of using
> EmitWarningsOnPlaceholders so that it will not be missed in the future
> modules.

I cannot find any such docs, so I add a comment to 
src/backend/utils/misc/guc.c.

> 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)
> 
> My point is, why should the "create extension" succeed with a WARNING
> when a wrong parameter related to it is set in the postgresql.conf
> file so that we don't see further problems as shown in (3). I think
> EmitWarningsOnPlaceholders should emit an error so that the extension
> will not get created in the first place if a wrong configuration
> parameter is set in the postgresql.conf file. Many times, users will
> not have access to server logs so it is good to fail the "create
> extension" statement.
> 
> Thoughts?
> 
> postgres=# create extension postgres_fdw ;
> ERROR:  unrecognized configuration parameter "postgres_fdw.XXX"
> postgres=#

I confirmed it too, and I think that's definitely a problem.
I also thought it would be a good idea to emit an ERROR as well as when 
an invalid normal GUC is set.
I didn't change the function name because it would affect many third 
party extensions.

I plan to change to emit an error when an invalid custom GUC is set in 
the SET or ALTER SYSTEM SET commands, but I haven't tackled this yet.
The patch as of now is attached.


-- 
Regards,

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