A friend asked me why his stored procedure wasn’t working. He was calling it from a program, so there was no way to test it, other than saying the update didn’t happen.
I looked at this procedure, it had one UPDATE statement. The meat of the code is shown below (I am paraphrasing):
CREATE OR REPLACE PROCEDURE upd_employee
SET employee_name = p_employee_name,
address = p_address,
created_dt = SYSDATE
employee_nbr = p_employee_nbr
WHEN OTHERS THEN
When I saw this, the first thing I noticed was that there is no proper error checking there. I see the EXCEPTION handling this. It’s natural to think that it will take care of all errors. Not so!
UPDATE Statement is an anamoly. When UPDATE statement is executed, it simply sets a variable SQL%ROWCOUNT to the # of rows updated and moves on. When UPDATE “fails” find any record to update, the ROWCOUNT will be 0. It is simply not an error as far as the database is concerned.
(This is unlike the case of a SELECT..INTO, where you get a NO_DATA_FOUND exception. Actually, not many developers do that. That’s for another story later).
Coming back to the issue, I suspected that the UPDATE did not happen and since there is no error handling, it “failed” silently.
First thing I did was to prove this theory. Looking at the procedure code, since it’s not checking for anything specific, I can write a crude test like so:
‘123 State St, Los Angeles’
I am essentially testing it as a blackbox like his program did. But, I am in an interactive session, where I can print using dbms_output.put_line to print debug messages. Now, you can see the real story. I am printing SQL%ROWCOUNT and it was 0 as expected.
For a quick fix, I just added the below line above the EXCEPTION and it did the job.
IF (SQL%ROWCOUNT = 0) THEN
But, in reality I will add lot more checks – including the types of the field etc. For application, instead of just RAISE, I would have added Raise_Application_Error function. Raise_Application_Error allows us to use -20000 .. -20999 as user error codes.
(I saw one program where they used -30000 which caused a brand new error to be generated. I will reserve that for another day!!). I may even add the below lines to be complete:
SHOW ERRORS PROCEDURE upd_employee
These are particularly helpful in SQL*Plus. First line makes sure the errors are printed and the EXIT leaves no ambiguity. (If you don’t have it, you will have to find ways to exit from OS level. See my earlier post for this).
And from a standards point of view, I would name the procedure sp_upd_employee to indicate it’s a stored procedure. I have a full standards document I made for a client years ago. I will post it some other time.
Before closing, I would like to mention couple of pet peeves of mine. One, a PL/SQL block must have the below syntax:
COMMIT should be part of the <body> and ROLLBACK should be in the EXCEPTION section. Having a COMMIT after the exception block is accepted syntactically, but it is WRONG! Because, he has a RAISE statement, it is never reached here, in case of any errors. But, the actual UPDATE statement is not COMMITted either!! (Not exactly true – it will actually COMMIT, but we are now leaving it to the mercy of SQL*Plus or any tool, which COMMITs when exiting the script, by default).
The other pet peeve of mine is the blank line inside SQL statements. I agree, it looks more readable, but wrong. PL/SQL ignores this, but if you run the SQL by itself, SQL*Plus will throw an error. I try to be consistent between SQL and PL/SQL to avoid surprises. If I really need a spacer, I add a comment line
— this is a comment
<rest of the SQL>;