Home > Software design >  Pl/sql not running through all my if statements
Pl/sql not running through all my if statements

Time:12-12

My script needs to do the following

if the dept is 10 and the salary is greater than 2000 give a 6% raise otherwise give a 7% raise. IF dept is 20 and the salary is greater than 2500 give a 5% raise otherwise give a 5.5% raise.

but the program is not running through all my if statements it is stopping at the first if statement and I am unsure of any other logic I can use to make it run through all the if statements.

SET SERVEROUTPUT ON

DECLARE
v_newsal emp.sal%TYPE;
v_sal emp.sal%TYPE;
v_deptno emp.deptno%TYPE;
CURSOR raise_cursor IS
  SELECT sal, deptno
    FROM emp;

BEGIN
OPEN raise_cursor;
fetch raise_cursor INTO v_sal, v_deptno;
LOOP
 IF v_deptno = 10 AND v_sal > 2000 THEN
   v_newsal := v_sal * 1.060;

 ELSE
  v_newsal := v_sal * 1.070;

 IF v_deptno = 20 AND v_sal > 2500 THEN
  v_newsal := v_sal * 1.050;

 ELSE
  v_newsal := v_sal * 1.055;

 END IF;
 END IF;


UPDATE emp SET
sal = v_newsal
WHERE deptno = v_deptno;

 EXIT;
END LOOP;
CLOSE raise_cursor;

END;
/
SET SERVEROUTPUT OFF

I have tried elsif statements and nested if statements and just regular if else statements but none seem to work.

CodePudding user response:

maybe something like this:

IF v_deptno = 10 THEN
   IF v_sal > 2500 THEN
       v_newsal := v_sal * 1.060;
   ELSE
       v_newsal := v_sal * 1.070;
   END IF;
END IF;
 
IF v_deptno = 20 THEN
   IF v_sal > 2500 THEN
      v_newsal := v_sal * 1.050;
   ELSE
      v_newsal := v_sal * 1.055;
   END IF;
END IF;

CodePudding user response:

Actually, you don't need inefficient row-by-row processing in a loop; use a single update statement with case expression:

update emp set
  sal = case when deptno = 10 then 
                  case when sal > 2000 then sal * 1.060
                       else sal * 1.070
                  end
             when deptno = 20 then
                  case when sal > 2500 then sal * 1.050
                       else sal * 1.055
                  end
             else sal
        end;
                  

CodePudding user response:

The issue is that you are opening the cursor, fetching a row, and then starting a loop, which you exit at the end of the first loop through.

The usual way of handling this type of cursor is: open the cursor, start the loop, fetch a row, exit the loop if a row wasn't found, or continue with the rest of your logic if a row was found, then go back to the start of the loop and repeat.

I'm assuming that your procedure is part of a learning exercise for dealing with cursors, because in real life, you'd be best off handling the logic in a single update statement like @Littlefoot suggested in their answer.

On that basis, your code should look like:

<snip>
OPEN raise_cursor;
LOOP
  fetch raise_cursor INTO v_sal, v_deptno;
  EXIT WHEN raise_cursor%NOTFOUND;

  <your logic>
END LOOP;
CLOSE raise_cursor;
<snip>

N.B. a cursor for loop (for <record> in <cursor> loop <logic> end loop;) is simpler and clearer to write, and also has inbuilt optimisations to convert from row-by-row processing to bulk row processing. It handles the opening, fetching and exiting the cursor for you, so you don't need to worry about it. However, as I've said, I'm assuming you're learning how to explicitly handle opening, looping, etc around a cursor.


Also, looking at the logic of your if statements, it doesn't make sense:

if the dept is 10 and the salary is greater than 2000 give a 6% raise otherwise give a 7% raise. IF dept is 20 and the salary is greater than 2500 give a 5% raise otherwise give a 5.5% raise.

Is that first "else" clause relating to just dept 10? And the same with the dept 20? Your current logic means that if you're in dept 20 with a lower than 2500 salary, or in any other dept, you'd get the 6% raise from the first query and then the 5.5% raise on top of that (sweet deal for the employees, not so much for the employer!).

If my understanding of the logic is correct, it should be something like:

IF v_deptno = 10 AND v_sal > 2000 THEN
  v_newsal := v_sal * 1.060;

ELSIF v_deptno = 10 THEN
  v_newsal := v_sal * 1.070;

ELSIF v_deptno = 20 AND v_sal > 2500 THEN
  v_newsal := v_sal * 1.050;

ELSIF v_deptno = 20 THEN -- maybe this clause should remain as the `ELSE` clause, if it applies to all other departments, etc
  v_newsal := v_sal * 1.055;

END IF

;

CodePudding user response:

You can use a single UPDATE statement and use a WHERE clause to only update employees in specific departments and a CASE expression to map the different department/salary ranges to the raise percentage:

UPDATE emp
SET sal = sal * CASE
                WHEN deptno = 10 AND sal > 2000 THEN 1.06
                WHEN deptno = 10                THEN 1.07
                WHEN deptno = 20 AND sal > 2500 THEN 1.05
                WHEN deptno = 20                THEN 1.055
                END
WHERE deptno IN (10,20)
  • Related