On Mon, Apr 3, 2023 at 9:15 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 3 Apr 2023, at 15:09, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Mon, Apr 3, 2023 at 8:46 AM Daniel Gustafsson <daniel@yesql.se> 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 follow. Do you propose that the GUC enables classes of event
> triggers, the default being "all" (or similar) and one would remove the type of
> EVT for which debugging is needed? That doesn't seem like a bad idea, just one
> that hasn't come up in the discussion (and I didn't think about).
Right. Although to be fair, that idea doesn't sound as good if we're
going to have settings other than "on" or "off". If this is just
disable_event_triggers = on | off, then why not flip the sense around
and make it event_triggers = off | on, just as we do for row_security?
But if we're going to allow specific types of event triggers to be
disabled, and we think it's likely that people will want to disable
one specific type of event trigger while leaving the others alone,
that might not be very convenient, because you could end up having to
list all the things you do want instead of the one thing you don't
want. On the third hand, in other contexts, I've often advocating for
giving options a positive sense (what are we going to do?) rather than
a negative sense (what are we not going to do?). For example, the
TIMING option to EXPLAIN was originally proposed with a name like
DISABLE_TIMING or something, and the value inverted, and I said let's
not do that. And similarly in other places. A case where I didn't
follow that principle is VACUUM (DISABLE_PAGE_SKIPPING) which now
seems like a wart to me. Why isn't it VACUUM (PAGE_SKIPPING) again
with the opposite value?
I'm not sure what the best thing to do is here, I just think it
deserves some thought.
--
Robert Haas
EDB: http://www.enterprisedb.com
