Simplify event trigger support checking functions

Started by Peter Eisentrautalmost 4 years ago3 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t46743
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 28, 2026 at 01:54 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t46743_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t46743_1 && git checkout t46743_1

Patchset v1 (message #1) is on t46743_1

Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

We have in event_trigger.c two functions
EventTriggerSupportsObjectType() and EventTriggerSupportsObjectClass()
that return whether a given object type/class supports event triggers.
Maybe there was a real differentiation there once, but right now it
seems to me that *all* object types/classes support event triggers,
except: (1) shared objects and (2) event triggers themselves. I think
we can write that logic more explicitly and compactly and don't have to
give the false illusion that there is a real choice to be made by the
implementer here.

The only drawback in terms of code robustness is that someone adding a
new shared object type would have to remember to add it to
EventTriggerSupportsObjectType(). Maybe we could add a "object type is
shared" function somehow, similar to IsSharedRelation(), to make that
easier. OTOH, someone doing that would probably go around and grep for,
say, OBJECT_TABLESPACE and find relevant places to update that way.

Thoughts?

Attachments:

t46743_1
0001-Simplify-event-trigger-support-checking-functions.patchtext/plain; charset=UTF-8; name=0001-Simplify-event-trigger-support-checking-functions.patchDownload+12-115
#2Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Simplify event trigger support checking functions

On Fri, Oct 7, 2022 at 8:10 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:

The only drawback in terms of code robustness is that someone adding a
new shared object type would have to remember to add it to
EventTriggerSupportsObjectType().

This doesn't seem like a good idea to me. If the function names give
the idea that you can decide whether new object types should support
event triggers or not, we could change them, or add better comments.
However, right now, you can't add a new object type and fail to update
these functions. With this change, that would become possible. And the
whole point of coding the functions in the way that they are was to
avoid that exact hazard. So I think we should leave the code the way
it is.

--
Robert Haas
EDB: http://www.enterprisedb.com

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#2)
Re: Simplify event trigger support checking functions

On 07.10.22 18:43, Robert Haas wrote:

On Fri, Oct 7, 2022 at 8:10 AM Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:

The only drawback in terms of code robustness is that someone adding a
new shared object type would have to remember to add it to
EventTriggerSupportsObjectType().

This doesn't seem like a good idea to me. If the function names give
the idea that you can decide whether new object types should support
event triggers or not, we could change them, or add better comments.
However, right now, you can't add a new object type and fail to update
these functions. With this change, that would become possible. And the
whole point of coding the functions in the way that they are was to
avoid that exact hazard.

I don't think just adding an entry to these functions is enough to make
event trigger support happen for an object type. There are other places
that need changing, and really you need to write a test to check it. So
I didn't think these functions provided any actual value. But I'm not
too obsessed about it if others feel differently.