Simplify event trigger support checking functions
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:
0001-Simplify-event-trigger-support-checking-functions.patchtext/plain; charset=UTF-8; name=0001-Simplify-event-trigger-support-checking-functions.patchDownload+12-115
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
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.