MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="=_9185a7fb380c371745f0250f1e79da52"
Content-Transfer-Encoding: 7bit
Date: Thu, 16 Dec 2021 16:31:03 +0900
From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Daniel Gustafsson <daniel@yesql.se>, PostgreSQL-development
 <pgsql-hackers@postgresql.org>
Subject: Re: Emit a warning if the extension's GUC is set incorrectly
In-Reply-To: 
 <CALj2ACUBKmDa35PmcfGZ==4rLNm4GW04D7o-ARkeipHijr5yuw@mail.gmail.com>
References: <524fa2c0a34f34b68fbfa90d0760d515@oss.nttdata.com>
 <681A95E0-0CCA-4C66-A12B-CE0D8D5076D7@yesql.se>
 <25643eaa81a69416317e85c2fd7d4c0a@oss.nttdata.com>
 <CALj2ACX9eDm9bPmcZDTGazEVrH+p7jUXfN0miBc_VVze_mjw9A@mail.gmail.com>
 <98d354d3b099d8e24c6b653336bd29d7@oss.nttdata.com>
 <CALj2ACUBKmDa35PmcfGZ==4rLNm4GW04D7o-ARkeipHijr5yuw@mail.gmail.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <4a2c5e69b98bf8ffbef2fad2f1850c97@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/4a2c5e69b98bf8ffbef2fad2f1850c97%40oss.nttdata.com>
Precedence: bulk

--=_9185a7fb380c371745f0250f1e79da52
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII;
 format=flowed

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
--=_9185a7fb380c371745f0250f1e79da52
Content-Transfer-Encoding: base64
Content-Type: text/x-diff;
 name=v4_Add_EmitWarningsOnPlaceholders.patch
Content-Disposition: attachment;
 filename=v4_Add_EmitWarningsOnPlaceholders.patch;
 size=4764

ZGlmZiAtLWdpdCBhL2NvbnRyaWIvYXV0aF9kZWxheS9hdXRoX2RlbGF5LmMgYi9jb250cmliL2F1
dGhfZGVsYXkvYXV0aF9kZWxheS5jCmluZGV4IDU4MjBhYzMyOGQuLmQxMWRkMWU0MTYgMTAwNjQ0
Ci0tLSBhL2NvbnRyaWIvYXV0aF9kZWxheS9hdXRoX2RlbGF5LmMKKysrIGIvY29udHJpYi9hdXRo
X2RlbGF5L2F1dGhfZGVsYXkuYwpAQCAtNjcsNiArNjcsOSBAQCBfUEdfaW5pdCh2b2lkKQogCQkJ
CQkJCU5VTEwsCiAJCQkJCQkJTlVMTCwKIAkJCQkJCQlOVUxMKTsKKworCUVtaXRXYXJuaW5nc09u
UGxhY2Vob2xkZXJzKCJhdXRoX2RlbGF5Iik7CisKIAkvKiBJbnN0YWxsIEhvb2tzICovCiAJb3Jp
Z2luYWxfY2xpZW50X2F1dGhfaG9vayA9IENsaWVudEF1dGhlbnRpY2F0aW9uX2hvb2s7CiAJQ2xp
ZW50QXV0aGVudGljYXRpb25faG9vayA9IGF1dGhfZGVsYXlfY2hlY2tzOwpkaWZmIC0tZ2l0IGEv
Y29udHJpYi9wZ190cmdtL3RyZ21fb3AuYyBiL2NvbnRyaWIvcGdfdHJnbS90cmdtX29wLmMKaW5k
ZXggZmIzODEzNWY3YS4uMDQwN2M3ZGQ2NCAxMDA2NDQKLS0tIGEvY29udHJpYi9wZ190cmdtL3Ry
Z21fb3AuYworKysgYi9jb250cmliL3BnX3RyZ20vdHJnbV9vcC5jCkBAIC0xMDAsNiArMTAwLDgg
QEAgX1BHX2luaXQodm9pZCkKIAkJCQkJCQkgTlVMTCwKIAkJCQkJCQkgTlVMTCwKIAkJCQkJCQkg
TlVMTCk7CisKKwlFbWl0V2FybmluZ3NPblBsYWNlaG9sZGVycygicGdfdHJnbSIpOwogfQogCiAv
KgpkaWZmIC0tZ2l0IGEvY29udHJpYi9wb3N0Z3Jlc19mZHcvb3B0aW9uLmMgYi9jb250cmliL3Bv
c3RncmVzX2Zkdy9vcHRpb24uYwppbmRleCA0OGM3NDE3ZTZlLi4zNjU1NTM5OGVjIDEwMDY0NAot
LS0gYS9jb250cmliL3Bvc3RncmVzX2Zkdy9vcHRpb24uYworKysgYi9jb250cmliL3Bvc3RncmVz
X2Zkdy9vcHRpb24uYwpAQCAtNDY5LDQgKzQ2OSw2IEBAIF9QR19pbml0KHZvaWQpCiAJCQkJCQkJ
ICAgTlVMTCwKIAkJCQkJCQkgICBOVUxMLAogCQkJCQkJCSAgIE5VTEwpOworCisJRW1pdFdhcm5p
bmdzT25QbGFjZWhvbGRlcnMoInBvc3RncmVzX2ZkdyIpOwogfQpkaWZmIC0tZ2l0IGEvY29udHJp
Yi9zZXBnc3FsL2hvb2tzLmMgYi9jb250cmliL3NlcGdzcWwvaG9va3MuYwppbmRleCAxOWEzZmZi
N2ZmLi40NGEwOWMzNWE3IDEwMDY0NAotLS0gYS9jb250cmliL3NlcGdzcWwvaG9va3MuYworKysg
Yi9jb250cmliL3NlcGdzcWwvaG9va3MuYwpAQCAtNDU1LDYgKzQ1NSw4IEBAIF9QR19pbml0KHZv
aWQpCiAJCQkJCQkJIE5VTEwsCiAJCQkJCQkJIE5VTEwpOwogCisJRW1pdFdhcm5pbmdzT25QbGFj
ZWhvbGRlcnMoInNlcGdzcWwiKTsKKwogCS8qIEluaXRpYWxpemUgdXNlcnNwYWNlIGFjY2VzcyB2
ZWN0b3IgY2FjaGUgKi8KIAlzZXBnc3FsX2F2Y19pbml0KCk7CiAKZGlmZiAtLWdpdCBhL3NyYy9i
YWNrZW5kL3V0aWxzL21pc2MvZ3VjLmMgYi9zcmMvYmFja2VuZC91dGlscy9taXNjL2d1Yy5jCmlu
ZGV4IGU5MWQ1YTNjZmQuLmQxMzZjYTEyZWEgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3V0aWxz
L21pc2MvZ3VjLmMKKysrIGIvc3JjL2JhY2tlbmQvdXRpbHMvbWlzYy9ndWMuYwpAQCAtNTQzMSwx
MSArNTQzMSwxOSBAQCB2YWxpZF9jdXN0b21fdmFyaWFibGVfbmFtZShjb25zdCBjaGFyICpuYW1l
KQogewogCWJvb2wJCXNhd19zZXAgPSBmYWxzZTsKIAlib29sCQluYW1lX3N0YXJ0ID0gdHJ1ZTsK
KwlpbnQgCQlwbGFjZWhvbGRlcl9sZW5ndGggPSAwOwogCiAJZm9yIChjb25zdCBjaGFyICpwID0g
bmFtZTsgKnA7IHArKykKIAl7CisJCXBsYWNlaG9sZGVyX2xlbmd0aCsrOwogCQlpZiAoKnAgPT0g
R1VDX1FVQUxJRklFUl9TRVBBUkFUT1IpCiAJCXsKKwkJCS8qIENoZWNrIGludmFsaWQgcGxhY2Vo
b2xkZXIgYW5kIGN1c3RvbSBwYXJhbWV0ZXIqLworCQkJY2hhciAqcGxhY2Vob2xkZXIgPSBtYWxs
b2Moc2l6ZW9mKGNoYXIpICogKHBsYWNlaG9sZGVyX2xlbmd0aCAtIDEpKTsKKwkJCXN0cm5jcHko
cGxhY2Vob2xkZXIsIG5hbWUsIHBsYWNlaG9sZGVyX2xlbmd0aCAtIDEpOworCQkJcGxhY2Vob2xk
ZXJbcGxhY2Vob2xkZXJfbGVuZ3RoIC0xXSA9ICdcMCc7CisJCQlFbWl0RXJyb3JzT25QbGFjZWhv
bGRlcnMocGxhY2Vob2xkZXIpOworCiAJCQlpZiAobmFtZV9zdGFydCkKIAkJCQlyZXR1cm4gZmFs
c2U7CS8qIGVtcHR5IG5hbWUgY29tcG9uZW50ICovCiAJCQlzYXdfc2VwID0gdHJ1ZTsKQEAgLTkz
MjIsNiArOTMzMCwxMyBAQCBEZWZpbmVDdXN0b21FbnVtVmFyaWFibGUoY29uc3QgY2hhciAqbmFt
ZSwKIAlkZWZpbmVfY3VzdG9tX3ZhcmlhYmxlKCZ2YXItPmdlbik7CiB9CiAKKy8qCisgKiBGb3Ig
aGlzdG9yaWNhbCBjb250ZXh0LCB0aGlzIGZ1bmN0aW9uIG5hbWUgaXMgRW1pdFdhcm5pbmdzT25Q
bGFjZWhvbGRlcnMsCisgKiBidXQgaXQgZW1pdHMgYW4gZXJyb3Igd2hlbiBhbiBpbnZhbGlkIGN1
c3RvbSBHVUMgaXMgc2V0LgorICoKKyAqIElmIHdlIHVzZSBEZWZpbmVDdXN0b21YWFhWYXJpYWJs
ZSwgaXQgaXMgcmVjb21tZW5kZWQgdGhhdCB0aGlzIGZ1bmN0aW9uIGlzCisgKiBhZGRlZC4KKyAq
Lwogdm9pZAogRW1pdFdhcm5pbmdzT25QbGFjZWhvbGRlcnMoY29uc3QgY2hhciAqY2xhc3NOYW1l
KQogewpAQCAtOTMzNiw3ICs5MzUxLDcgQEAgRW1pdFdhcm5pbmdzT25QbGFjZWhvbGRlcnMoY29u
c3QgY2hhciAqY2xhc3NOYW1lKQogCQkJc3RybmNtcChjbGFzc05hbWUsIHZhci0+bmFtZSwgY2xh
c3NMZW4pID09IDAgJiYKIAkJCXZhci0+bmFtZVtjbGFzc0xlbl0gPT0gR1VDX1FVQUxJRklFUl9T
RVBBUkFUT1IpCiAJCXsKLQkJCWVyZXBvcnQoV0FSTklORywKKwkJCWVyZXBvcnQoRVJST1IsCiAJ
CQkJCShlcnJjb2RlKEVSUkNPREVfVU5ERUZJTkVEX09CSkVDVCksCiAJCQkJCSBlcnJtc2coInVu
cmVjb2duaXplZCBjb25maWd1cmF0aW9uIHBhcmFtZXRlciBcIiVzXCIiLAogCQkJCQkJCXZhci0+
bmFtZSkpKTsKZGlmZiAtLWdpdCBhL3NyYy9wbC90Y2wvcGx0Y2wuYyBiL3NyYy9wbC90Y2wvcGx0
Y2wuYwppbmRleCBlMTE4Mzc1NTlkLi4xMzg5YWEwOTQzIDEwMDY0NAotLS0gYS9zcmMvcGwvdGNs
L3BsdGNsLmMKKysrIGIvc3JjL3BsL3RjbC9wbHRjbC5jCkBAIC00NzQsNiArNDc0LDggQEAgX1BH
X2luaXQodm9pZCkKIAkJCQkJCQkgICBQR0NfU1VTRVQsIDAsCiAJCQkJCQkJICAgTlVMTCwgTlVM
TCwgTlVMTCk7CiAKKwlFbWl0V2FybmluZ3NPblBsYWNlaG9sZGVycygicGx0Y2x1Iik7CisKIAlw
bHRjbF9wbV9pbml0X2RvbmUgPSB0cnVlOwogfQogCmRpZmYgLS1naXQgYS9zcmMvdGVzdC9tb2R1
bGVzL2RlbGF5X2V4ZWN1dGlvbi9kZWxheV9leGVjdXRpb24uYyBiL3NyYy90ZXN0L21vZHVsZXMv
ZGVsYXlfZXhlY3V0aW9uL2RlbGF5X2V4ZWN1dGlvbi5jCmluZGV4IGIzZDA4NDFiYTguLjhlYzYy
M2FjNTIgMTAwNjQ0Ci0tLSBhL3NyYy90ZXN0L21vZHVsZXMvZGVsYXlfZXhlY3V0aW9uL2RlbGF5
X2V4ZWN1dGlvbi5jCisrKyBiL3NyYy90ZXN0L21vZHVsZXMvZGVsYXlfZXhlY3V0aW9uL2RlbGF5
X2V4ZWN1dGlvbi5jCkBAIC05MSw2ICs5MSw4IEBAIF9QR19pbml0KHZvaWQpCiAJCQkJCQkJTlVM
TCwKIAkJCQkJCQlOVUxMKTsKIAorCUVtaXRXYXJuaW5nc09uUGxhY2Vob2xkZXJzKCJkZWxheV9l
eGVjdXRpb24iKTsKKwogCS8qIEluc3RhbGwgb3VyIGhvb2sgKi8KIAlwcmV2X3BsYW5uZXJfaG9v
ayA9IHBsYW5uZXJfaG9vazsKIAlwbGFubmVyX2hvb2sgPSBkZWxheV9leGVjdXRpb25fcGxhbm5l
cjsKZGlmZiAtLWdpdCBhL3NyYy90ZXN0L21vZHVsZXMvc3NsX3Bhc3NwaHJhc2VfY2FsbGJhY2sv
c3NsX3Bhc3NwaHJhc2VfZnVuYy5jIGIvc3JjL3Rlc3QvbW9kdWxlcy9zc2xfcGFzc3BocmFzZV9j
YWxsYmFjay9zc2xfcGFzc3BocmFzZV9mdW5jLmMKaW5kZXggNmIwYTNkYjEwNC4uM2JhMzNlNTAx
YyAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvbW9kdWxlcy9zc2xfcGFzc3BocmFzZV9jYWxsYmFjay9z
c2xfcGFzc3BocmFzZV9mdW5jLmMKKysrIGIvc3JjL3Rlc3QvbW9kdWxlcy9zc2xfcGFzc3BocmFz
ZV9jYWxsYmFjay9zc2xfcGFzc3BocmFzZV9mdW5jLmMKQEAgLTQ4LDYgKzQ4LDkgQEAgX1BHX2lu
aXQodm9pZCkKIAkJCQkJCQkgICBOVUxMLAogCQkJCQkJCSAgIE5VTEwsCiAJCQkJCQkJICAgTlVM
TCk7CisKKwlFbWl0V2FybmluZ3NPblBsYWNlaG9sZGVycygic3NsX3Bhc3NwaHJhc2UiKTsKKwog
CWlmIChzc2xfcGFzc3BocmFzZSkKIAkJb3BlbnNzbF90bHNfaW5pdF9ob29rID0gc2V0X3JvdDEz
OwogfQpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvbW9kdWxlcy93b3JrZXJfc3BpL3dvcmtlcl9zcGku
YyBiL3NyYy90ZXN0L21vZHVsZXMvd29ya2VyX3NwaS93b3JrZXJfc3BpLmMKaW5kZXggMGI2MjQ2
Njc2Yi4uYWRiMDJkOGNiOCAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvbW9kdWxlcy93b3JrZXJfc3Bp
L3dvcmtlcl9zcGkuYworKysgYi9zcmMvdGVzdC9tb2R1bGVzL3dvcmtlcl9zcGkvd29ya2VyX3Nw
aS5jCkBAIC0zMjIsNiArMzIyLDggQEAgX1BHX2luaXQodm9pZCkKIAkJCQkJCQkgICAwLAogCQkJ
CQkJCSAgIE5VTEwsIE5VTEwsIE5VTEwpOwogCisJRW1pdFdhcm5pbmdzT25QbGFjZWhvbGRlcnMo
Indvcmtlcl9zcGkiKTsKKwogCS8qIHNldCB1cCBjb21tb24gZGF0YSBmb3IgYWxsIG91ciB3b3Jr
ZXJzICovCiAJbWVtc2V0KCZ3b3JrZXIsIDAsIHNpemVvZih3b3JrZXIpKTsKIAl3b3JrZXIuYmd3
X2ZsYWdzID0gQkdXT1JLRVJfU0hNRU1fQUNDRVNTIHwK
--=_9185a7fb380c371745f0250f1e79da52--