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