Re: GUC for temporarily disabling event triggers

2023-09-25 Thread Daniel Gustafsson
> On 25 Sep 2023, at 09:52, Daniel Gustafsson wrote: > >> On 25 Sep 2023, at 09:50, Michael Paquier wrote: >> >> On Mon, Sep 25, 2023 at 09:19:35AM +0200, Daniel Gustafsson wrote: >>> Fair enough, although I used "fire" instead of "run" which is consistent >>> with >>> the event trigger

Re: GUC for temporarily disabling event triggers

2023-09-25 Thread Daniel Gustafsson
> On 25 Sep 2023, at 09:50, Michael Paquier wrote: > > On Mon, Sep 25, 2023 at 09:19:35AM +0200, Daniel Gustafsson wrote: >> Fair enough, although I used "fire" instead of "run" which is consistent with >> the event trigger documentation. > > Okay by me. Great, I'll go ahead and apply this

Re: GUC for temporarily disabling event triggers

2023-09-25 Thread Michael Paquier
On Mon, Sep 25, 2023 at 09:19:35AM +0200, Daniel Gustafsson wrote: > Fair enough, although I used "fire" instead of "run" which is consistent with > the event trigger documentation. Okay by me. -- Michael signature.asc Description: PGP signature

Re: GUC for temporarily disabling event triggers

2023-09-25 Thread Daniel Gustafsson
> On 25 Sep 2023, at 01:35, Michael Paquier wrote: > > On Fri, Sep 22, 2023 at 05:29:01PM +0200, Daniel Gustafsson wrote: >> Since the main driver for this is to reduce the usage/need for single-user >> mode >> I also reworded the patch slightly. Instead of phrasing this as an >> alternative

Re: GUC for temporarily disabling event triggers

2023-09-24 Thread Michael Paquier
095b057377cd05bf5 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 22 Sep 2023 16:39:07 +0200 Subject: [PATCH v9 1/2] Add GUC for temporarily disabling event triggers In order to troubleshoot misbehaving or buggy event triggers, the documented advice is to enter single-user mode. In

Re: GUC for temporarily disabling event triggers

2023-09-22 Thread Daniel Gustafsson
> On 7 Sep 2023, at 21:02, Robert Haas wrote: > > On Thu, Sep 7, 2023 at 1:57 AM Michael Paquier wrote: >> + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE >> >> I am a bit surprised by these two additions. Setting this GUC at >> file-level can be useful, as is documenting it in the

Re: GUC for temporarily disabling event triggers

2023-09-07 Thread Robert Haas
On Thu, Sep 7, 2023 at 1:57 AM Michael Paquier wrote: > + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE > > I am a bit surprised by these two additions. Setting this GUC at > file-level can be useful, as is documenting it in the control file if > it provides some control of how a statement

Re: GUC for temporarily disabling event triggers

2023-09-06 Thread Michael Paquier
On Wed, Sep 06, 2023 at 10:23:55PM +0200, Daniel Gustafsson wrote: > > On 6 Sep 2023, at 16:22, Robert Haas wrote: >> I usually prefer to give things a positive sense, talking about >> whether things are enabled rather than disabled. I'd do event_triggers >> = off | on, like we have for

Re: GUC for temporarily disabling event triggers

2023-09-06 Thread Daniel Gustafsson
> On 6 Sep 2023, at 16:22, Robert Haas wrote: > On Wed, Sep 6, 2023 at 4:50 AM Daniel Gustafsson wrote: >> Fair enough, how about disable_event_trigger instead as per the attached? > > I usually prefer to give things a positive sense, talking about > whether things are enabled rather than

Re: GUC for temporarily disabling event triggers

2023-09-06 Thread Robert Haas
On Wed, Sep 6, 2023 at 4:50 AM Daniel Gustafsson wrote: > > On 5 Sep 2023, at 17:29, Robert Haas wrote: > > On Tue, Sep 5, 2023 at 8:12 AM Daniel Gustafsson wrote: > > >> The attached version of the patch replaces it with a boolean flag for > >> turning > >> off all event triggers, and I also

Re: GUC for temporarily disabling event triggers

2023-09-06 Thread Daniel Gustafsson
> On 5 Sep 2023, at 17:29, Robert Haas wrote: > On Tue, Sep 5, 2023 at 8:12 AM Daniel Gustafsson wrote: >> The attached version of the patch replaces it with a boolean flag for turning >> off all event triggers, and I also renamed it to the debug_xxx "GUC >> namespace" >> which seemed more

Re: GUC for temporarily disabling event triggers

2023-09-05 Thread Robert Haas
On Tue, Sep 5, 2023 at 8:12 AM Daniel Gustafsson wrote: > > On 6 Apr 2023, at 00:06, Michael Paquier wrote: > > If there is no strong > > case for more than a boolean for now, simpler is better. > > The attached version of the patch replaces it with a boolean flag for turning > off all event

Re: GUC for temporarily disabling event triggers

2023-09-05 Thread Daniel Gustafsson
> On 6 Apr 2023, at 00:06, Michael Paquier wrote: > If there is no strong > case for more than a boolean for now, simpler is better. The attached version of the patch replaces it with a boolean flag for turning off all event triggers, and I also renamed it to the debug_xxx "GUC namespace" which

Re: GUC for temporarily disabling event triggers

2023-04-05 Thread Michael Paquier
On Wed, Apr 05, 2023 at 10:43:23AM -0400, Tom Lane wrote: > Robert Haas writes: >> Maybe we should back up and ask why we need more than "on" and "off". >> If somebody is using this feature in any form more than very >> occasionally, they should really go home and reconsider their database >>

Re: GUC for temporarily disabling event triggers

2023-04-05 Thread Daniel Gustafsson
> On 5 Apr 2023, at 16:30, Robert Haas wrote: > > On Wed, Apr 5, 2023 at 4:57 AM Daniel Gustafsson wrote: >>> - Being able to write a list of event triggers working would be much >>> more interesting than just individual elements. >>> - There may be an argument for negated patterns? Say, >>> a

Re: GUC for temporarily disabling event triggers

2023-04-05 Thread Tom Lane
Robert Haas writes: > Maybe we should back up and ask why we need more than "on" and "off". > If somebody is using this feature in any form more than very > occasionally, they should really go home and reconsider their database > schema. +1 ... this seems perhaps overdesigned.

Re: GUC for temporarily disabling event triggers

2023-04-05 Thread Robert Haas
On Wed, Apr 5, 2023 at 4:57 AM Daniel Gustafsson wrote: > > - Being able to write a list of event triggers working would be much > > more interesting than just individual elements. > > - There may be an argument for negated patterns? Say, > > a "!sql_drop,!ddl_command_start" would cause sql_drop

Re: GUC for temporarily disabling event triggers

2023-04-05 Thread Daniel Gustafsson
> On 5 Apr 2023, at 10:10, Michael Paquier wrote: > > On Mon, Apr 03, 2023 at 11:35:14PM +0200, Daniel Gustafsson wrote: >> Yeah. The patch as it stands allow for disabling specific types rather than >> all-or-nothing, which is why the name was "ignore". > > FWIW, I agree with Robert's points

Re: GUC for temporarily disabling event triggers

2023-04-05 Thread Michael Paquier
On Mon, Apr 03, 2023 at 11:35:14PM +0200, Daniel Gustafsson wrote: > Yeah. The patch as it stands allow for disabling specific types rather than > all-or-nothing, which is why the name was "ignore". FWIW, I agree with Robert's points here: - disable_event_triggers or ignore_event_triggers = off

Re: GUC for temporarily disabling event triggers

2023-04-03 Thread Daniel Gustafsson
> On 3 Apr 2023, at 16:09, Robert Haas wrote: > On Mon, Apr 3, 2023 at 9:15 AM Daniel Gustafsson wrote: >>> On 3 Apr 2023, at 15:09, Robert Haas wrote: >>> I continue to think it's odd that the sense of this is inverted as >>> compared with row_security. >> >> I'm not sure I follow. Do you

Re: GUC for temporarily disabling event triggers

2023-04-03 Thread Robert Haas
On Mon, Apr 3, 2023 at 9:15 AM Daniel Gustafsson wrote: > > On 3 Apr 2023, at 15:09, Robert Haas wrote: > > On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson wrote: > >> All comments above addressed in the attached v5, thanks for review! > > > > I continue to think it's odd that the sense of

Re: GUC for temporarily disabling event triggers

2023-04-03 Thread Daniel Gustafsson
> On 3 Apr 2023, at 15:09, Robert Haas wrote: > > On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson wrote: >> All comments above addressed in the attached v5, thanks for review! > > I continue to think it's odd that the sense of this is inverted as > compared with row_security. I'm not sure I

Re: GUC for temporarily disabling event triggers

2023-04-03 Thread Robert Haas
On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson wrote: > All comments above addressed in the attached v5, thanks for review! I continue to think it's odd that the sense of this is inverted as compared with row_security. -- Robert Haas EDB: http://www.enterprisedb.com

Re: GUC for temporarily disabling event triggers

2023-04-03 Thread Daniel Gustafsson
> On 2 Apr 2023, at 21:48, Justin Pryzby wrote: > + gettext_noop("Disable event triggers for the duration > of the session."), > > Why does is it say "for the duration of the session" ? > > It's possible to disable ignoring, and within the same session. > GUCs are

Re: GUC for temporarily disabling event triggers

2023-04-02 Thread Justin Pryzby
On Mon, Mar 06, 2023 at 01:24:56PM +0100, Daniel Gustafsson wrote: > > On 27 Jan 2023, at 15:00, Mikhail Gribkov wrote: > > > There is a complete framework for disabling various types of the event > > triggers separately, but, the list of valid GUC values only include 'all' > > and 'none'. Why

Re: GUC for temporarily disabling event triggers

2023-04-02 Thread Daniel Gustafsson
> On 7 Mar 2023, at 16:02, Mikhail Gribkov wrote: > * The patch does what it intends to do; > * The implementation way is clear; > * All test are passed; > * No additional problems catched - at least by my eyes; > > I think it can be marked as Ready for Committer This patch has been RFC for

Re: GUC for temporarily disabling event triggers

2023-03-07 Thread Mikhail Gribkov
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed I like it now. * The patch does what it intends to do; *

Re: GUC for temporarily disabling event triggers

2023-03-06 Thread Daniel Gustafsson
> On 27 Jan 2023, at 15:00, Mikhail Gribkov wrote: > There is a complete framework for disabling various types of the event > triggers separately, but, the list of valid GUC values only include 'all' and > 'none'. Why not adding support for all the event trigger types separately? > Everything

Re: GUC for temporarily disabling event triggers

2023-01-27 Thread Mikhail Gribkov
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed Hi Daniel, I have reviewed the patch and I liked it (well I

Re: GUC for temporarily disabling event triggers

2023-01-12 Thread Ted Yu
On Thu, Jan 12, 2023 at 12:26 PM Daniel Gustafsson wrote: > > On 11 Jan 2023, at 17:38, vignesh C wrote: > > > > On Tue, 29 Nov 2022 at 18:16, Daniel Gustafsson wrote: > >> > >>> On 3 Nov 2022, at 21:47, Daniel Gustafsson wrote: > >> > >>> The patch adds a new GUC, ignore_event_trigger with

Re: GUC for temporarily disabling event triggers

2023-01-12 Thread Daniel Gustafsson
> On 11 Jan 2023, at 17:38, vignesh C wrote: > > On Tue, 29 Nov 2022 at 18:16, Daniel Gustafsson wrote: >> >>> On 3 Nov 2022, at 21:47, Daniel Gustafsson wrote: >> >>> The patch adds a new GUC, ignore_event_trigger with two option values, 'all' >>> and 'none' (the login event patch had

Re: GUC for temporarily disabling event triggers

2023-01-11 Thread vignesh C
On Tue, 29 Nov 2022 at 18:16, Daniel Gustafsson wrote: > > > On 3 Nov 2022, at 21:47, Daniel Gustafsson wrote: > > > The patch adds a new GUC, ignore_event_trigger with two option values, 'all' > > and 'none' (the login event patch had 'login' as well). > > The attached v2 fixes a small bug

Re: GUC for temporarily disabling event triggers

2022-11-29 Thread Daniel Gustafsson
> On 3 Nov 2022, at 21:47, Daniel Gustafsson wrote: > The patch adds a new GUC, ignore_event_trigger with two option values, 'all' > and 'none' (the login event patch had 'login' as well). The attached v2 fixes a small bug which caused testfailures the CFBot. -- Daniel Gustafsson

GUC for temporarily disabling event triggers

2022-11-03 Thread Daniel Gustafsson
In the thread discussing the login event trigger patch it was argued that we want to avoid recommending single-user mode for troubleshooting tasks, and a GUC for temporarily disabling event triggers was proposed. Since the login event trigger patch lost momentum, I've broken out the GUC part