Skip to content

Commit 2663e3e

Browse files
committed
Fix #91 - Changed G-7730 to more generic and added G-7740 for specific PK case not supported by PLSQL cop
1 parent cf358cd commit 2663e3e

File tree

3 files changed

+76
-25
lines changed

3 files changed

+76
-25
lines changed
Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
# G-7730: Avoid multiple DML events per trigger if primary key is assigned in trigger.
1+
# G-7730: Avoid multiple DML events per trigger.
22

3-
!!! warning "Major"
4-
Efficiency, Reliability
3+
!!! tip "Minor"
4+
Maintainability, Testability
55

66
## Reason
77

8-
If a trigger makes assignment to the primary key anywhere in the trigger code, that causes the session firing the trigger to take a lock on any child tables with a foreign key to this primary key. Even if the assignment is in for example an `if inserting` block and the trigger is fired by an `update` statement, such locks still happen unnecessarily. The issue is avoided by having one trigger for the insert containing the primary key assignment, and another trigger for the update. Or even better by handling the insert assignment as ´default on null´ clauses, so that only an `on update` trigger is needed.
8+
Rather than a single trigger handling multiple DML events with separated blocks of `if inserting`, `if updating` and `if deleting`, modularity by individual triggers per DML event helps maintaining and testing the code. If most of the code is common for either DML event (only small pieces of code are individual) consider an exception to the rule and allow `if inserting`, `if updating` and `if deleting` blocks, or alternatively gather the common code in a procedure and let individual triggers handle the individual pieces of code plus call the procedure with the common code.
9+
10+
If the trigger makes assignment to a primary key and there are child tables with a foreign key referring to this primary key, the database can make undesirable table locks. If such is the case, you should always use individual triggers. See [G-7740](../../../../4-language-usage/7-stored-objects/7-triggers/g-7740) for details.
911

1012
## Example (bad)
1113

@@ -15,8 +17,7 @@ before insert or update
1517
on departments for each row
1618
begin
1719
if inserting then
18-
:new.department_id := department_seq.nextval;
19-
:new.created_date := sysdate;
20+
:new.created_date := sysdate;
2021
end if;
2122
if updating then
2223
:new.changed_date := sysdate;
@@ -25,15 +26,14 @@ end;
2526
/
2627
```
2728

28-
## Example (better)
29+
## Example (good)
2930

3031
``` sql
3132
create or replace trigger dept_br_i
3233
before insert
3334
on departments for each row
3435
begin
35-
:new.department_id := department_seq.nextval;
36-
:new.created_date := sysdate;
36+
:new.created_date := sysdate;
3737
end;
3838
/
3939

@@ -45,18 +45,3 @@ begin
4545
end;
4646
/
4747
```
48-
49-
## Example (good)
50-
51-
``` sql
52-
alter table department modify department_id default on null department_seq.nextval;
53-
alter table department modify created_date default on null sysdate;
54-
55-
create or replace trigger dept_br_u
56-
before update
57-
on departments for each row
58-
begin
59-
:new.changed_date := sysdate;
60-
end;
61-
/
62-
```
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# G-7740: Never handle multiple DML events per trigger if primary key is assigned in trigger.
2+
3+
!!! warning "Major"
4+
Efficiency, Reliability
5+
6+
!!! missing "Unsupported in PL/SQL Cop Validators"
7+
We cannot identify what the primary key column(s) are to check if assignment to a primary key is taking place in the trigger.
8+
9+
## Reason
10+
11+
If a trigger makes assignment to the primary key anywhere in the trigger code, that causes the session firing the trigger to take a lock on any child tables with a foreign key to this primary key. Even if the assignment is in for example an `if inserting` block and the trigger is fired by an `update` statement, such locks still happen unnecessarily. The issue is avoided by having one trigger for the insert containing the primary key assignment, and another trigger for the update. Or even better by handling the insert assignment as ´default on null´ clauses, so that only an `on update` trigger is needed.
12+
13+
## Example (bad)
14+
15+
``` sql
16+
create or replace trigger dept_br_iu
17+
before insert or update
18+
on departments for each row
19+
begin
20+
if inserting then
21+
:new.department_id := department_seq.nextval;
22+
:new.created_date := sysdate;
23+
end if;
24+
if updating then
25+
:new.changed_date := sysdate;
26+
end if;
27+
end;
28+
/
29+
```
30+
31+
## Example (good)
32+
33+
``` sql
34+
create or replace trigger dept_br_i
35+
before insert
36+
on departments for each row
37+
begin
38+
:new.department_id := department_seq.nextval;
39+
:new.created_date := sysdate;
40+
end;
41+
/
42+
43+
create or replace trigger dept_br_u
44+
before update
45+
on departments for each row
46+
begin
47+
:new.changed_date := sysdate;
48+
end;
49+
/
50+
```
51+
52+
## Example (best)
53+
54+
``` sql
55+
alter table department modify department_id default on null department_seq.nextval;
56+
alter table department modify created_date default on null sysdate;
57+
58+
create or replace trigger dept_br_u
59+
before update
60+
on departments for each row
61+
begin
62+
:new.changed_date := sysdate;
63+
end;
64+
/
65+
```

docs/9-appendix/appendix.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ n/a | 7460 | Try to define your packaged/standalone function deterministic if ap
118118
76 | 7510 | Always prefix ORACLE supplied packages with owner schema name. | Major | | | | | | | ✘ |
119119
77 | 7710 | Avoid cascading triggers. | Major | | | ✘ | | | | | ✘
120120
n/a | 7720 | Never use multiple UPDATE OF in trigger event clause. | Blocker | | | ✘ | | ✘ | | | ✘
121-
n/a | 7730 | Avoid multiple DML events per trigger if primary key is assigned in trigger. | Major | | ✘ | | | ✘ | | |
121+
n/a | 7730 | Avoid multiple DML events per trigger. | Minor | | | ✘ | | | | | ✘
122+
n/a | 7740 | Never handle multiple DML events per trigger if primary key is assigned in trigger. | Major | | ✘ | | | ✘ | | |
122123
n/a | 7810 | Never use SQL inside PL/SQL to read sequence numbers (or SYSDATE). | Major | | ✘ | ✘ | | | | |
123124
78 | 8110 | Never use SELECT COUNT(*) if you are only interested in the existence of a row. | Major | | ✘ | | | | | |
124125
n/a | 8120 | Never check existence of a row to decide whether to create it or not. | Major | | ✘ | | | ✘ | | |

0 commit comments

Comments
 (0)