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

2022-02-21 Thread Florin Irion
Il lun 21 feb 2022, 20:12 Tom Lane ha scritto: > I wrote: > > Florin Irion writes: > >> For what it's worth, I agree with throwing an ERROR if the placeholder > is > >> unrecognized. Initially, I didn't want to change too much the liberty of > >> setting any placeholder, but mainly to not go

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

2022-02-21 Thread Tom Lane
I wrote: > Florin Irion writes: >> For what it's worth, I agree with throwing an ERROR if the placeholder is >> unrecognized. Initially, I didn't want to change too much the liberty of >> setting any placeholder, but mainly to not go unnoticed. > I also think that this is probably a good change

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

2022-02-18 Thread Tom Lane
Florin Irion writes: > Il giorno ven 18 feb 2022 alle ore 10:58 Tom Lane ha > scritto: >> Here's a delta patch (meant to be applied after reverting cab5b9ab2) >> that does things like that. It fixes the parallel-mode problem ... >> so do we want to tighten things up this much? > Thank you for

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

2022-02-18 Thread Florin Irion
Il giorno ven 18 feb 2022 alle ore 10:58 Tom Lane ha scritto: > I wrote: > > As a stopgap to turn the farm green again, I am going to revert > > 75d22069e as well as my followup patches. If we don't want to > > give up on that idea altogether, we have to find some way to > > suppress the

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

2021-12-28 Thread Tom Lane
I wrote: > As a stopgap to turn the farm green again, I am going to revert > 75d22069e as well as my followup patches. If we don't want to > give up on that idea altogether, we have to find some way to > suppress the chatter from parallel workers. I wonder whether > it would be appropriate to go

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

2021-12-27 Thread Tom Lane
I wrote: > Concretely, I think we should do the attached, which removes much of > 75d22069e and does the check at the point of placeholder creation. I pushed that, and along the way moved the test case to be beside the existing tests concerning custom GUC names, rather than appended at the end of

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

2021-12-23 Thread Shinya Kato
On 2021/12/22 3:33, Tom Lane wrote: I wrote: 0003 looks to me like it was superseded by 75d22069e. I do not particularly like that patch though; it seems both inefficient and ill-designed. 0003 is adding a check in an equally bizarre place. Why isn't add_placeholder_variable the place to

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

2021-12-21 Thread Shinya Kato
On 2021-12-22 02:23, Tom Lane wrote: Kyotaro Horiguchi writes: At Mon, 20 Dec 2021 21:05:23 +0900, Shinya Kato wrote in We should use EmitWarningsOnPlaceholders when we use DefineCustomXXXVariable. I don't think there is any room for debate. Unfortunately, pltcl.c defines variables both

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

2021-12-21 Thread Tom Lane
I wrote: > 0003 looks to me like it was superseded by 75d22069e. I do not > particularly like that patch though; it seems both inefficient > and ill-designed. 0003 is adding a check in an equally bizarre > place. Why isn't add_placeholder_variable the place to deal > with this? Also, once we

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

2021-12-21 Thread Tom Lane
Kyotaro Horiguchi writes: > At Mon, 20 Dec 2021 21:05:23 +0900, Shinya Kato > wrote in >> We should use EmitWarningsOnPlaceholders when we use >> DefineCustomXXXVariable. >> I don't think there is any room for debate. > Unfortunately, pltcl.c defines variables both in pltcl and pltclu but >

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

2021-12-20 Thread Kyotaro Horiguchi
At Fri, 17 Dec 2021 11:25:22 +0900, Shinya Kato wrote in > On 2021-12-17 01:55, Fujii Masao wrote: > > I'm still not sure why you were thinking that ERROR is more proper > > here. > > Since I get an ERROR when I set the wrong normal GUCs, I thought I > should also get an ERROR when I set the

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

2021-12-20 Thread Kyotaro Horiguchi
At Mon, 20 Dec 2021 21:05:23 +0900, Shinya Kato wrote in > - v6-01-Add-EmitWarningsOnPlaceholders.patch > We should use EmitWarningsOnPlaceholders when we use > DefineCustomXXXVariable. > I don't think there is any room for debate. Unfortunately, pltcl.c defines variables both in pltcl and

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

2021-12-20 Thread Shinya Kato
On 2021-12-17 15:42, Peter Eisentraut wrote: On 17.12.21 03:25, Shinya Kato wrote: For now, I'v attached the patch that fixed the compilation error. I think it would be good if you could split the uncontroversial new EmitErrorsOnPlaceholders() calls into a separate patch. And please add an

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

2021-12-16 Thread Peter Eisentraut
On 17.12.21 03:25, Shinya Kato wrote: For now, I'v attached the patch that fixed the compilation error. I think it would be good if you could split the uncontroversial new EmitErrorsOnPlaceholders() calls into a separate patch. And please add an explanation what exactly the rest of the

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

2021-12-16 Thread Fujii Masao
On 2021/12/17 11:25, Shinya Kato wrote: For now, I'v attached the patch that fixed the compilation error. Thanks for updating the patch! I confirmed that the compilation was completed successfully with this new patch. But the regression test failed. You can see that Patch Tester [1] also

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

2021-12-16 Thread Shinya Kato
On 2021-12-17 01:55, Fujii Masao wrote: On 2021/12/16 16:31, Shinya Kato wrote: 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)

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

2021-12-16 Thread Fujii Masao
On 2021/12/16 16:31, Shinya Kato wrote: 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

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

2021-12-15 Thread Shinya Kato
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

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

2021-11-16 Thread Bharath Rupireddy
On Tue, Nov 16, 2021 at 2:50 PM Shinya Kato wrote: > > 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

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

2021-11-16 Thread Shinya Kato
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

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

2021-11-14 Thread Bharath Rupireddy
On Mon, Nov 15, 2021 at 6:33 AM Shinya Kato 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

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

2021-11-14 Thread Shinya Kato
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

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

2021-11-14 Thread Daniel Gustafsson
> On 14 Nov 2021, at 11:03, Shinya Kato wrote: > If wrong GUCs of auth_delay, pg_trgm, postgres_fdw and sepgsql are described > in postgresql.conf, a warning is not emitted unlike pg_stat_statements, > auto_explain and pg_prewarm. > So, I improved it by adding EmitWarningsOnPlaceholders. > An

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

2021-11-14 Thread Shinya Kato
Hi hackers, If wrong GUCs of auth_delay, pg_trgm, postgres_fdw and sepgsql are described in postgresql.conf, a warning is not emitted unlike pg_stat_statements, auto_explain and pg_prewarm. So, I improved it by adding EmitWarningsOnPlaceholders. An example output is shown below. ---