scope of raise exception, handling your own exceptions in PLSQL code
I have this procedure:
create or replace PROCEDURE CONVERTE
IS
CURSOR oldemployees IS
SELECT *
FROM emp1
WHERE data_saida= NULL;
new_ndep emp1.num_dep%type;
bi_inexistente EXCEPTION;
dep_inexistente EXCEPTION;
employeeNr emp1.num_empregado%type;
BEGIN
FOR old_emp IN oldemployees
LOOP
employeeNr:= old_emp.num_empregado;
if (old_emp.bi = NULL) then
raise bi_inexistente;
else
IF (old_emp.num_dep>20) THEN
SELECT ndep_novo INTO new_ndep FROM Converte_dep WHERE ndep_antigo= old_emp.num_dep;
elsif (old_emp.num_dep = NULL) then
new_ndep:= 0;
raise dep_inexistente;
end if;
INSERT INTO EMP2 VALUES (old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old开发者_JAVA百科_emp.data_saida, new_ndep);
COMMIT;
end if;
end loop;
EXCEPTION
when bi_inexistente then
INSERT INTO ERROS VALUES(employeeNr, 'BI Inexistente');
COMMIT;
when dep_inexistente then
INSERT INTO ERROS VALUES(employeeNr, 'Departamento Inexistente');
COMMIT;
end;
I want to do INSERT INTO EMP2 VALUES (old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old_emp.data_saida, new_ndep); even after the raising dep_inexistente, but after reading oracle's reference, I'm a little bit confused; Basically, when it's null, I want to not do that insert, otherwise I want to insert, even when department number is null (which I turn to 0).
So, is the code getting it right or how should I raise my exceptions or handle pre-defined exceptions for my case?
I want to do INSERT INTO EMP2 VALUES (old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old_emp.data_saida, new_ndep); even after the raising dep_inexistente
The trick is to raise that exception after doing the inserts. Raised exceptions are effectively GOTO
statements - the flow of control zips straight to the EXCEPTIONS block. In the following rewrite I have used your setting of new_dep as a signal to raise the exception. You may be aware of some other business logic which invalidates this approach (i.e. there is some legitimate reason why a record would have a department zero). In which case you will need to set a flag instead.
create or replace PROCEDURE CONVERTE IS
CURSOR oldemployees IS
SELECT *
FROM emp1
WHERE data_saida= NULL;
new_ndep emp1.num_dep%type;
bi_inexistente EXCEPTION;
dep_inexistente EXCEPTION;
employeeNr emp1.num_empregado%type;
BEGIN
FOR old_emp IN oldemployees
LOOP
employeeNr:= old_emp.num_empregado;
if (old_emp.bi is NULL) then
raise bi_inexistente;
else
if (old_emp.num_dep>20) THEN
SELECT ndep_novo INTO new_ndep FROM Converte_dep WHERE ndep_antigo= old_emp.num_dep;
elsif (old_emp.num_dep is NULL) then
new_ndep:= 0;
end if;
INSERT INTO EMP2 VALUES (old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old_emp.data_saida, new_ndep);
COMMIT;
if new_ndep = 0 then
raise dep_inexistente;
end if;
end if;
end loop;
EXCEPTION
when bi_inexistente then
INSERT INTO ERROS VALUES(employeeNr, 'BI Inexistente');
COMMIT;
when dep_inexistente then
INSERT INTO ERROS VALUES(employeeNr, 'Departamento Inexistente');
COMMIT;
end;
Three things about your general approach:
- Any exception will shortcircuit the LOOP. No further rows will be processed
- Because you are commiting within the LOOP it may be hard to rerun the program, because you won't easily be able to pickup from where you left off.
- Commiting inside the loop can create problems with ORA-1555 or ORA-1002 errors, especially if this is a long running query.
edit
Actually your code raises a great many questions regarding the procedural logic. Far more than I would wish to go into here. The three I listed above are the general "best practice" issues but the detailed logic of the conditional flow looks iffy. But then I don't know the business rules you are implementing.
I don't think exceptions should be used as an inelegant GOTO statement. If you want to structure your code you could use procedures (and subprocedures). If the work is done at one place in the code just use the RETURN statement. Catch exceptions only when that makes sense.
There is an error in your code: old_emp.num_dep = NULL
can't work, it is always false.
Suppose it would have been old_emp.num_dep IS NULL
, then I think your code would not work according to your intenion. The exception would be raised bypassing the INSERT INTO EMP2.
If this were my code, and the logic is such that you can decide that it is not a real error to insert into EMP2 in case the department is missing, I would not raise an exception. You are not losing that information either, as you can always see there were missing departments (namely for every emp with 0 as department)
BTW is there a particular reason for using 0 for the department? Why not just use NULL
? Apparently you have decided that it is ok to have employees without department, NULL
is a fair representation of that.
If you insist that it is actually an error for the emp to miss a department, but still feel it is ok to insert the EMP anyway, I'd consider writing it like this:
IF ... THEN
... -- ok
END IF;
INSERT INTO EMP2 VALUES (
old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old_emp.data_saida,
NVL(new_ndep, 0)
);
IF new_ndep IS NULL THEN
raise dep_inexistente;
END IF;
I urge you to add some comments in the code though, because if I would find code like what I wrote above, I would probably suspect a bug.
So, If I keep exceptions it would be like this:
create or replace PROCEDURE CONVERTE IS
CURSOR oldemployees IS
SELECT *
FROM emp1
WHERE data_saida= NULL;
new_ndep emp1.num_dep%type;
bi_inexistente EXCEPTION;
dep_inexistente EXCEPTION;
employeeNr emp1.num_empregado%type;
BEGIN
FOR old_emp IN oldemployees
LOOP
employeeNr:= old_emp.num_empregado;
if (old_emp.bi is NULL) then
raise bi_inexistente;
else
if (old_emp.num_dep>20) THEN
SELECT ndep_novo INTO new_ndep FROM Converte_dep WHERE ndep_antigo= old_emp.num_dep;
else
INSERT INTO EMP2 VALUES (old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old_emp.data_saida,nvl(old_emp.num_dep,0));
end if;
if new_ndep is NULL then
raise dep_inexistente;
end if;
end if;
end loop;
EXCEPTION
when bi_inexistente then
INSERT INTO ERROS VALUES(employeeNr, 'BI Inexistente');
COMMIT;
when dep_inexistente then
INSERT INTO ERROS VALUES(employeeNr, 'Departamento Inexistente');
COMMIT;
end;
or I could just do what is told, WITHOUT raising exceptions; but I still have to use a cursor.
create or replace
PROCEDURE CONVERTE2 IS
CURSOR oldemployees IS
SELECT *
FROM emp1
WHERE data_saida= NULL;
new_ndep emp1.num_dep%type;
bi_inexistente EXCEPTION;
dep_inexistente EXCEPTION;
employeeNr emp1.num_empregado%type;
v_error_code NUMBER:=0;
v_error_message VARCHAR2(255);
BEGIN
FOR old_emp IN oldemployees
LOOP
employeeNr:= old_emp.num_empregado;
if (old_emp.bi is NULL) then
INSERT INTO ERROS VALUES(employeeNr, 'BI Inexistente');
else
if (old_emp.num_dep>20) THEN
SELECT ndep_novo INTO new_ndep FROM Converte_dep WHERE ndep_antigo= old_emp.num_dep;
else
INSERT INTO EMP2 VALUES (old_emp.bi, old_emp.nome, old_emp.morada, old_emp.data_entrada, old_emp.data_saida,nvl(old_emp.num_dep,0));
end if;
if new_ndep is NULL then
INSERT INTO ERROS VALUES(employeeNr, 'Departamento Inexistente');
end if;
end if;
end loop;
COMMIT;
EXCEPTION
When others Then
ROLLBACK;
/*eventually log something into erro table*/
end;
How would you rewrite it then, so it doesn't look so "iffy"? It's kind of messy, I have to admit. Anyway, at least you gave me very pratical insights. I would like to see some better aproaches, if you want to, I'm interested.
精彩评论