'INSERT RETURNING INTO' can clean up old coding methods

Just remembered in Oracle 11g there are two lots of  functionality that can help with existing code like this


SELECT ex_sequence_number.NEXTVAL
INTO v_ex_sequence_number
FROM dual;
p_batch_number := v_ex_sequence_number; -- out parameter


INSERT INTO upload_validated (
sequence_number,
widget_cd,
comments)
VALUES (
v_ex_sequence_number,
p_widget_cd,
'This is the old way to do it');


First we don't need to do the select any more as we can just say


INSERT INTO upload_validated (
sequence_number,
widget_cd,
comments)
VALUES (
ex_sequence_number.NEXTVAL,-- no need to do the old way
p_widget_cd,
'This is the old way to do it');

But because we also need to set the out parameter (p_batch_number using the same sequence value) we also need to use the returning into clause for the insert.

INSERT INTO upload_validated (

sequence_number,

widget_cd,

comments)

VALUES (

ex_sequence_number.NEXTVAL,-- no need to do the old way

p_widget_cd,

'This is the old way to do it')

RETURNING sequence_number

INTO p_batch_number

Which is a much nicer and concise way to do it.

Compile time warnings

There isn't really a Lint like tool for PL/SQL. The closest you can get is enabling compile time warnings.

In the session where you will do the compile issue the following
ALTER SESSION SET PLSQL_WARNINGS='ENABLE:ALL';

And to switch it back off
ALTER SESSION SET PLSQL_WARNINGS='DISABLE:ALL';

If use the test function below as an example we can see a bunch of things that look wrong.

FUNCTION test_func
  ( p_emp_id IN emp.emp_id%TYPE)
RETURN varchar2 IS
CURSOR c_emp IS
SELECT surname
FROM emp
WHERE emp_id = p_emp_id;
v_surname emp.surname%TYPE;
v_not_used NUMBER;
BEGIN 
IF 1 = 2 THEN
OPEN c_emp;
FETCH c_emp INTO v_surname;
RETURN v_surname;
ELSE
dbms_output.put_line('This will never appear');
END IF;
EXCEPTION
WHEN OTHERS THEN
RAISE;
END;

This is the feedback we get from the compile time warnings

[1:1] PLW-05018: unit TEST_FUNC omitted optional AUTHID clause; default value DEFINER used
[4:2] PLW-06006: uncalled procedure "C_EMP" is removed.
[12:3] PLW-06002: Unreachable code
[1:1] PLW-05005: subprogram TEST_FUNC returns without value at line 21


We get 4 things returned from the compile time warnings. 
line 1 - We didn't define AUTHID and Oracle thinks it is good practice to always explicitly define it. By default, if it is not set explicitly, it will be set to DEFINER rights. If you want to remove this warning then either switch the warning off or define the AUTHID in the code.
line 4 - the cursor c_emp is never used so it is removed
line 12 - there is code that is never executed inside the if 1 = 2
line 21 - no return value and it is a function. The line number can't really make sense for this one so it uses the last line.


These are all good things to know but there seems to be a few other things it would be nice to be told too.
If we use a simpler function then these issues stand out a bit more

FUNCTION test_func2
  ( p_emp_id IN emp.emp_id%TYPE)
RETURN varchar2 IS
CURSOR c_emp IS
SELECT surname
FROM emp
WHERE emp_id = p_emp_id;
v_surname emp.surname%TYPE;
v_not_used NUMBER;
BEGIN
OPEN c_emp;
FETCH c_emp INTO v_surname;
RETURN v_surname;
dbms_output.put_line('Never to be seen');
EXCEPTION
WHEN OTHERS THEN
RAISE;
END;


Gives us these warnings


[1:1] PLW-05018: unit TEST_FUNC omitted optional AUTHID clause; default value DEFINER used
[14:2] PLW-06002: Unreachable code

So we again see unreachable code as expected for the dbms_output line that will never be hit, as it is after a return, but it would also be nice to know.

1. The variable v_not_used is declared but never used
2. The cursor c_emp is opened but never closed

Number 1 is not a big deal but a Lint checker would pick this up. There are other ways to do this using PL/SCOPE

Number 2 would be nice to know as leaving cursors open can cause lots of issues like exceeding max cursors.

It would be super nice if it told you also that the c_emp should be closed in the exception section so that it is caught if something goes wrong. I've been working on my own PL/SQL code to try and pick this up in source code checks but that I will leave that to another post when it gets fleshed out a bit more.