Wednesday, September 13, 2006

Just, Why?

I'm currently debugging a procedure of the form illustrated below, lovingly recreated using the ubiquitous scott.emp table. I'm also taking suggestions on what might've been going through the head of the person who wrote it.

RENAME emp TO emp_table
     FROM emp_table
   ON emp
   new_emp                       emp_table%ROWTYPE;
   old_emp                       emp_table%ROWTYPE;
   new_emp.empno       := :NEW.empno;
   new_emp.ename       := :NEW.ename;
   new_emp.job         := :NEW.job;
   new_emp.mgr         := :NEW.mgr;
   new_emp.hiredate    := :NEW.hiredate;
   new_emp.sal         := :NEW.sal;
   new_emp.comm        := :NEW.comm;
   new_emp.deptno      := :NEW.deptno;
   old_emp.empno       := :OLD.empno;
   old_emp.ename       := :OLD.ename;
   old_emp.job         := :OLD.job;
   old_emp.mgr         := :OLD.mgr;
   old_emp.hiredate    := :OLD.hiredate;
   old_emp.sal         := :OLD.sal;
   old_emp.comm        := :OLD.comm;
   old_emp.deptno      := :OLD.deptno;
   update_emp (old_emp, new_emp);

   old_emp                             emp_table%ROWTYPE
  ,new_emp                             emp_table%ROWTYPE
   IF old_emp.empno != new_emp.empno
      RETURN;   --can't update primary key attribute
   END IF;

   IF     old_emp.empno IS NOT NULL
      AND new_emp.empno IS NULL
      DELETE FROM emp_table
            WHERE empno = old_emp.empno;

   END IF;

   IF    (    old_emp.ename IS NULL
          AND new_emp.ename IS NOT NULL)
      OR (    old_emp.ename IS NOT NULL
          AND new_emp.ename != old_emp.ename)
      UPDATE emp_table
         SET ename = new_emp.ename
       WHERE empno = old_emp.empno;
   END IF;
   -- ..
   -- ..
   -- ..
   IF    (    old_emp.deptno IS NULL
          AND new_emp.deptno IS NOT NULL)
      OR (    old_emp.deptno IS NOT NULL
          AND new_emp.deptno != old_emp.deptno)
      UPDATE emp_table
         SET deptno = new_emp.deptno
       WHERE empno = old_emp.empno;
   END IF;


Anonymous said...

That's easy. They were told to redesign the database schema without changing existing client code.

It looks like they only got as far as putting a proxy system in place without actually changing the database tables.

Covenant said...

everbody knows that this sort of thing shouldn't happen. that's why we have ORM tools like hibernate. ;P

Scott Swank said...

I'm pretty torn about Hibernate. On the one hand, it lets me ignore 90% of the SQL and focus on the 10% that matters. On the other hand, it can be painfully oblique -- especially its exceptions.

Anonymous said...

What was going through the programmers mind? Who knows, but maybe they should lay off the drugs!

Nigel said...

Looks to me like this was an attempt at "optimization" - only updating the columns that have actually been changed. Just a shame that if you change 20 columns, you get 20 separate updates... so more like "pessimization" really.

Covenant said...

personally, im still on the fence on the whole ORM thing. part of me screams that its just a bad idea. but on the other hand, I can see it being very useful in some simple cases, but then, if the cases are simple what's wrong with just hand coding some data access mechanisms....

James Padfield said...

I can't help feeling the 'IF old_emp.empno != new_emp.empno THEN RETURN' would be nicely complemented by a classic 'WHEN OTHERS THEN NULL', or perhaps a bit of code to raise a random error on the third day after bill run when the month has an 'A' in it.

I'm probably biased, but my experience of Hibernate goes like this...

Web Developer: The database contains wrong/no data.
Oracle Developer: I see data, what SP are you calling / SQL are you running?
Web Developer: I'm using Hibernate, it doesn't show you the SQL.
Oracle Developer: Sorry, can't really help too much then. which point the Web Developer retires to his desk to spend a week transfixed without food or water, emerging some time later to announce that he has fixed a 'problem' with the Hibernate mappings.

Which is presumably where the name Hibernate comes from?

Anonymous said...

"...emerging some time later to announce that he has fixed a 'problem' with the Hibernate mappings."

...And isn't the "fix" normally to replace the rows and columns in the tables with Bee-Lobs that the application "manages" itself?

Scott Swank said...

Padders, If the developer doesn't know how to see the SQL that Hibernate generates he/she ought to be shown the door. Or or you could take pity on the poor bastard and type "hibernate show sql" into google.

Covenant, I'm torn as well. Though I have to admit that just now it's serving me well. I have a WebServiceCommand class that I've sub-classed in many ways to represent the various dialogs we need. Then I can store them all via Hibernate and fetch them when the web service is up (not often enough thank you very much) and simply call "execute()" on each of them. Hibernate handles all of the sub-classing for me such that I don't have to keep track of what execute() does for any of the various commands. 'Course catch me in a week and I'll be cursing it again.

Niall said...

I might be missing something but my guess would be that this code predates "rename table" and has never been changed.

William Robertson said...

From the days when you could create an INSTEAD-OF trigger but not rename a table, you mean?

Covenant said...

What's wrong with storing all your data in xml in Bee-Lobs anyway? If they are in an Oracle table, then surely that's relational enough for most people!

Jeff Hunter said...

At lunch, they were thinking:
"One more beer won't hurt, will it?"

Anonymous said...

That's something my *project manager* could have written, and checked into the source repository. Without telling the developers of his changes, of course (nor testing it, btw)