public inbox for [email protected]  
help / color / mirror / Atom feed
From: Mikhail Gribkov <[email protected]>
To: [email protected]
Cc: Daniel Gustafsson <[email protected]>
Subject: Re: GUC for temporarily disabling event triggers
Date: Fri, 27 Jan 2023 14:00:11 +0000
Message-ID: <167482801126.1114.1095311628314619931.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <CALte62x7c9R0ohKcQSr5N9PFB-D6pCb_THhwWzTscR7ZB6OHvg@mail.gmail.com>
References: <[email protected]>
	<CALte62x7c9R0ohKcQSr5N9PFB-D6pCb_THhwWzTscR7ZB6OHvg@mail.gmail.com>

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 did liked it already since it was a part of login trigger patch previously). 
All tests are passed and the manual experiments with all types of event triggers are also passed.

Everything is fine and I think It can be marked as Ready for Committer, although I have one final question.
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 is already there in the patch; the only thing needed is expanding couple of enums. It's cheap in terms of code size and even cheaper in terms of performance. And moreover - it would be a good example for anyone adding new trigger types.

The new status of this patch is: Waiting on Author


view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: GUC for temporarily disabling event triggers
  In-Reply-To: <167482801126.1114.1095311628314619931.pgcf@coridan.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox