On Mon, Nov 15, 2021 at 6:33 AM Shinya Kato
<Shinya11.Kato@oss.nttdata.com> wrote:
>
> On 2021-11-15 04:50, Daniel Gustafsson wrote:
> > Seems reasonable on a quick skim, commit da2c1b8a2 did a similar
> > roundup back
> > in 2009 but at the time most of these didn't exist (pg_trgm did but
> > didn't have
> > custom option back then). There is one additional callsite defining
> > custom
> > variables in src/pl/tcl/pltcl.c which probably should get this
> > treatment as
> > well, it would align it with the pl/perl counterpart.
> >
> > I'll have a closer look and test tomorrow.
> Thank you for the review!
> I have missed src/pl/tcl/pltcl.c, so I created the new patch.
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.
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 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=#
Regards,
Bharath Rupireddy.