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

score 5 · Accepted Answer · 2012-11-15T08:09:02.460

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.

With this one it works, but we are required to do it using a cursor. But if i use a cursor, the updated salary is wrong. — Katherine, Nov 15 '12 at 07:51
Seems strange that you are required to use a cursor? If you still need to investigate the cause of your problem I should recommend you to add some trace messages into the code (with e.g. dbms_output) to be able to see what what is done by the updates. This way you should be able to find out the reason why you get the incorrect value. — Peter Å, Nov 15 '12 at 08:04
@Katherine: then tell your teacher that the assignment is teaching *very* bad coding techniques. I think your problem is caused by the fact that you are leaving the loop at the wrong position. See my edit. — , Nov 15 '12 at 08:07
@Katherine: I wouldn't "tell" him/her. But why don't you ask him/her if a simple update would be more efficient? — , Nov 15 '12 at 08:23
@a_horse_with_no_name because i think she want us to practice using cursors.. and also our current topic is cursors. When i show my code to her she tell me that the problem is in my if else statement that's why i got confused and decided to ask suggestions here.. :) — Katherine, Nov 15 '12 at 08:27

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

1 Answers1