PL/SQL-updating salary of an employee, the amount of increase in salary was wrong

Question

I need to update the salary of employees from department 40 and 70. All employees from department 40 will have a 10% increase while employees from department 70 will have 15% increase.

I have 1 employee from department 70 who has a salary of 10000 so he will have a 15% increase. I expect his salary to become 11500, but it becomes 13225. I can't understand why. The employee from department 40 have the correct salary increase, only this one from department 70 is wrong.

here is the pl/sql block..

SET serveroutput ON
DECLARE

  CURSOR cur_emp
  IS
    SELECT * FROM employees WHERE department_id = 40 OR department_id = 70;
  rec_emp cur_emp%rowtype;
BEGIN
  OPEN cur_emp;
  LOOP
    FETCH cur_emp INTO rec_emp;
    IF rec_emp.department_id = 40 THEN
      UPDATE employees
      SET salary                = salary + (salary * 0.1)
      WHERE employee_id         = rec_emp.employee_id;
    elsif rec_emp.department_id = 70 THEN
      UPDATE employees
      SET salary        = salary + (salary * 0.15)
      WHERE employee_id = rec_emp.employee_id;
    END IF;
    EXIT
  WHEN cur_emp%notfound;
  END LOOP;
  CLOSE cur_emp;
END;
/

Could anyone help me figure out the problem with this one? thanks

Answer 1

No need for a stored procedure:

update employees
   set salary = case 
                  when department_id = 40 then salary * 1.10
                  when department_id = 70 then salary * 1.15
                  else salary -- not strictly necessary. just to make sure.
                end
where department_id in (40,70);

If you insist on doing it the slow way (a loop in PL/SQL) using an UPDATE ... WHERE CURRENT OF would probably be faster than "unrelated" updates.

The real problem with your code is that you are leaving the loop "too late". Even if the cursor does not return anything after the fetch, you still do the update. You should put the EXIT WHEN ... before the IF clause and the update.

DECLARE

  CURSOR cur_emp
  IS
    SELECT * FROM employees WHERE department_id = 40 OR department_id = 70;
  rec_emp cur_emp%rowtype;
BEGIN
  OPEN cur_emp;
  LOOP
    FETCH cur_emp INTO rec_emp;

    EXIT WHEN cur_emp%notfound; -- **** leave the loop right here, BEFORE doing the update *****

    IF rec_emp.department_id = 40 THEN
      UPDATE employees
      SET salary                = salary + (salary * 0.1)
      WHERE employee_id         = rec_emp.employee_id;
    elsif rec_emp.department_id = 70 THEN
      UPDATE employees
      SET salary        = salary + (salary * 0.15)
      WHERE employee_id = rec_emp.employee_id;
    END IF;

  END LOOP;
  CLOSE cur_emp;
END;
/

A more efficient way would be to use an updatable cursor though:

DECLARE

  CURSOR cur_emp
  IS
    SELECT department_id, salary 
    FROM employees 
    WHERE department_id in (40,70)
    FOR UPDATE OF salary;

  rec_emp cur_emp%rowtype;
  new_sal number(12,2);
BEGIN
  OPEN cur_emp;
  LOOP
    FETCH cur_emp INTO rec_emp;

    EXIT WHEN cur_emp%NOTFOUND;

    IF rec_emp.department_id = 40 THEN
      new_sal := rec_emp.salary * 1.10;
    elsif rec_emp.department_id = 70 THEN
      new_sal := rec_emp.salary * 1.15;
    END IF;

    UPDATE employees
       SET salary  = new_sal
    WHERE CURRENT OF cur_emp;

  END LOOP;
  CLOSE cur_emp;
END;
/

The use of the WHERE CURRENT OF will actually reveal you error more clearly, because the loop will fail with an invalid rowid if you put the exit after the update.