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

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-12-16 07:31:03
Message-ID: 4a2c5e69b98bf8ffbef2fad2f1850c97@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.
However, it also gives me an error when
---
postgres=# SET abc.a TO on;
SET
postgres=# SET abc.b TO on;
2021-12-16 16:22:20.351 JST [2489236] ERROR: unrecognized configuration
parameter "abc.a"
2021-12-16 16:22:20.351 JST [2489236] STATEMENT: SET abc.b TO on;
ERROR: unrecognized configuration parameter "abc.a"
---

I know that some people do not think this is good.
Therefore, can I leave this problem alone or is there another better
way?

> For backward compatibility you can retain the function
> EmitWarningsOnPlaceholders as-is and have another similar function
> that emits an error:
>
> In guc.c, have something like below:
> static void
> check_conf_params(const char *className, bool emit_error)
> {
> /* have the existing code of EmitWarningsOnPlaceholders here */
> ereport(emit_error ? ERROR : WARNING,
> (errcode(ERRCODE_UNDEFINED_OBJECT),
> errmsg("unrecognized configuration parameter
> \"%s\"",
> var->name)));
> }
>
> void
> EmitErrorOnPlaceholders(const char *className)
> {
> check_conf_params(className, true);
> }
>
> void
> EmitWarningsOnPlaceholders(const char *className)
> {
> check_conf_params(className, false);
> }
>

Thank you for your advise.
According to [1], we used the same function name, but the warning level
was INFO.
Therefore, I think it is OK to use the same function name.

[1]
https://www.postgresql.org/message-id/flat/200901051634.n05GYNr06169%40momjian.us#1d045374f014494e4b40a4862a000723

--
Regards,

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

Attachment Content-Type Size
v4_Add_EmitWarningsOnPlaceholders.patch text/x-diff 4.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message houzj.fnst@fujitsu.com 2021-12-16 08:25:35 RE: row filtering for logical replication
Previous Message houzj.fnst@fujitsu.com 2021-12-16 07:27:02 RE: parallel vacuum comments