From: | Shinya Kato <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Emit a warning if the extension's GUC is set incorrectly |
Date: | 2021-11-16 09:20:33 |
Message-ID: | 98d354d3b099d8e24c6b653336bd29d7@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v3_Add_EmitWarningsOnPlaceholders.patch | text/x-diff | 4.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Etsuro Fujita | 2021-11-16 09:55:45 | Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit |
Previous Message | Dilip Kumar | 2021-11-16 08:49:43 | Re: row filtering for logical replication |