Thread: 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?

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.