Feature #4076
remove empty triggers during conversion
0%
History
#1 Updated by Greg Shah about 7 years ago
One of our customers adds trigger procedures for every permanent database table, even if the trigger is empty. In fact, many of these triggers are empty. They are there in case someone wants to add something later. In such a scenario, no schema changes are needed. The program can simply be edited and updated in source control.
The down side is that there are a very large number of extra triggers implemented, which will certainly have a performance cost in both the original and FWD systems.
The customer asked if we can remove empty triggers during the conversion. Of course we can, it is just a matter of effort. They are specifically asking about schema triggers, but the concept really applies to all trigger types.
This will work fine so long as there are no side-effects of invoking an empty trigger. Or at least as long as there are no side effects upon which the application can rely.
The implementation would have to handle the following:
- Detecting
- For UI triggers and session-level DB triggers, we would look at the contents of
TRIGGER_BLOCK/BLOCK/and make sure that there are no child nodes. - For schema triggers, the entire external procedure must be empty except for the
TRIGGER_PROCEDUREstatement, which must be ignored. - We should do this calculation in
annotations/early_annotation.xmls, leaving behind an annotation with the result. - We should add reports using this annotation.
- For UI triggers and session-level DB triggers, we would look at the contents of
- Bypassing
- For UI triggers and session-level DB triggers, I think we can hide the associated ON statement in annotations.
- Schema triggers will probably require a bit more work.
- Optimally, we would ignore these completely and not generate a resulting class.
- We will definitely need to avoid registering these as part of the conversion.
Can anyone think of a reason this won't work?
What other implementation details or issues do we need to consider?
#2 Updated by Ovidiu Maxiniuc about 7 years ago
From the POV of schema triggers only: yes, this should work.
I have a question. We can detect if the trigger file is empty quite early, in fixups. Are we able to put a file on 'ignore' and not process it in any way (annotation/convert/brew) after this?
#3 Updated by Greg Shah about 7 years ago
I have a question. We can detect if the trigger file is empty quite early, in fixups. Are we able to put a file on 'ignore' and not process it in any way (annotation/convert/brew) after this?
Yes, I was thinking of this approach as well. We could implement some kind of registry for the ignore list and then the ConversionDriver would need to refresh its list of files in between each execution of the PatternEngine.
The processing of the schema triggers would still need to be made safe so that we don't complain about a missing trigger procedure if it is empty. But otherwise, I guess the approach would be pretty safe.
#4 Updated by Ovidiu Maxiniuc about 5 years ago
In my initial implementation of schema triggers, the linking between the event and the invoked trigger class/method was decided at conversion time. Recently, Adrian added support for resolving the trigger procedure/class at runtime, based on PROPATH system variable, as OE does. This means that the programmer can change the active trigger for a table/field by simply changing the order of paths inside the variable.
This makes me think of a scenario where some programmer/hacker use two different triggers (with same name). S/He can dynamically suspend the trigger by raising priority of the path with empty triggers. If we detect and drop these, then the lookup will continue searching past the expected path and find and use the next matching trigger. Maybe I am too evil-ish but a similar scenario can happen when an unexpected procedure is called because the designed NO-OP procedure was removed.
The UI triggers do not pose this problem since they are hardcoded in the converted source.
#5 Updated by Greg Shah about 5 years ago
Maybe I am too evil-ish but a similar scenario can happen when an unexpected procedure is called because the designed NO-OP procedure was removed.
No, you are not being evil. I agree it is a real issue.
OK, I think there is still a way we can make this better. We have to resolve the empty triggers. Perhaps we could emit a method override public boolean isEmpty() { return true; } which can report that should bypass execution. A parent class can implement public boolean isEmpty() { return false; }. Is there any reason this would be an issue?
My idea is that at least we will avoid a bunch or setup and tear-down for no reason. On the other hand, if there is some side-effect for executing an empty trigger, then we may need to execute it after all.
#6 Updated by Greg Shah about 1 year ago
The dangerous case here is when there are more than one trigger that exists with the same filename (and operating on the same table name) AND not all of them are empty. In that case, the runtime propath will determine which is actually active and it is necessary to leave behind those empty triggers so that the behavior is the same as the original system. This isn't just about malicious intent. All such triggers have to be present at conversion time and the programmer must have control over the source repo and/or the inputs to conversion AS WELL AS the runtime propath. But even without malicious intent, this may just be a "valid/intended feature" used by programmers to "disable" triggers by inserting an empty one in the propath.
But we can detect this at conversion time, because we know the filenames and the referenced tables. So we can safely drop any empty triggers that have no duplicate filename or where the duplicate filename is operating on a different table name. For most applications, this may eliminate all empty triggers. If we log a warning at conversion time when we are forced to "save" an empty trigger, then developers can easily find this case later and fix it by eliminating the use of this "feature".
This seems like a good "win" in terms of performance even if it is minor for most customers.
#7 Updated by Ovidiu Maxiniuc about 1 year ago
After some thoughts, I think we can drop the empty triggers. If the 4GL programmer really wants to disable a normally written trigger, (s)he can do that by creating a non-empty NOP trigger with a statement like
IF True THEN LEAVE.(or maybe something simpler?). This way, FWD conversion will keep the file (even if the if statement might be dropped due to dead code checks), but it will not contain any logic code.#8 Updated by Greg Shah about 1 year ago
Ovidiu Maxiniuc wrote:
After some thoughts, I think we can drop the empty triggers. If the 4GL programmer really wants to disable a normally written trigger, (s)he can do that by creating a non-empty NOP trigger with a statement like[...](or maybe something simpler?). This way, FWD conversion will keep the file (even if the
ifstatement might be dropped due to dead code checks), but it will not contain any logic code.
The problem is that it might show up as a bug that we then have to diagnose before realizing that it was caused by dropping the empty triggers. Why not just put the safer version in now, and avoid the extra work later? I guess in most cases, the problem should be rare but I don't want to waste time later on it.
#9 Updated by Ovidiu Maxiniuc about 1 year ago
OK. I agree.