While working on the join elimination patch, I was going through the
trigger code and found quite a bit of nastiness in regard to naming
and variable repurposing related to the addition of replication roles
in 8.3. The most obvious issue is that tgenabled was switched from a
bool to char to support replication roles. From a naming standpoint,
the term "enabled" generally implies boolean and is fairly
consistently used as such in other functions within the core. My
initial preference would be to return tgenabled to its original
boolean for use only in enabling/disabling triggers. Then, I'd
probably add another boolean entry (tgreplica?) for use in determining
whether the trigger should be fired on origin/local or replica.
Otherwise, the naming of EnableDisableTrigger and friends seems a bit
contradictory due to the fact that it has the ability to convert a
trigger into a replica trigger. Similarly, I can't see any reason for
keeping the structure member name the same, especially when the change
from bool to char broke backward compatibility anyway. Thoughts?
As I wasn't sure whether anyone agrees with my distaste for
repurposing tgenabled as mentioned above, I have attached is a patch
which minimally corrects the function comment for EnableDisableTrigger
where fires_when is concerned.
--
Jonah H. Harris, Senior DBA
myYearbook.com