scope of raise exception, handling your own exceptions in PLSQL code

Question

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_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?

Answer 1

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.

Answer 2

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:

1. Any exception will shortcircuit the LOOP. No further rows will be processed
2. 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.
3. 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.

Answer 3

Hi newbAna,

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.

Answer 4

@Roland Bouman This code is an exercise to convert data from one table to another; the restrictions are:

1- in the new table,emp2, the primary key is bi and if it's null we have to insert in erros table the information about it (it doesn't specify to handle an exception for that, but it's pretty common);

2- All the department numbers >20 had been changed, based on old and new value in Convert_dep(ndep_antigo,ndep_novo) table. If the employee, emp, doesn't have a department, it has to be inserted but with department number = 0. We have again to inserrt the error saying that the department doesn't exist. We also that, in case the employee has a department, it's is valid: <20 ou existing in table Converte_dep (for example, dep nr 20 turns into 40, dep nr 30 turns into 55, and so on).

It's weird, I have to agree, but was just an exercise. Thanks for your repply!

Answer 5

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.