Tuesday, March 01, 2016

Designing PL/SQL Programs

When I started out, in COBOL, structured programming was king. COBOL programs tended to be lengthy and convoluted. Plus GOTO statements. We needed program desire to keep things under control.

So I noticed the absence of design methodologies when I moved into Oracle. At first it didn't seem to be a problem. SQL was declarative and self-describing, and apparently didn't need designing. Forms was a 4GL and provided its own structure. And PL/SQL? Well that was just a glue, and the programs were so simple.

Then one day I was debugging several hundred lines of PL/SQL somebody had written, and struggling to figure out what was going on. So I drew a flow chart of the IF branches and WHILE loops. Obvious really, but if the original author had done that they would have realised that the program had an ELSE branch which could never be chosen; more than one hundred lines of code which would never execute.

Let me sleep()


Good design is hard to define: in fact, good design is often unobtrusive. It's bad design we notice, because it generates friction and hinders our progress. By way of illustration, here is a poor design choice in Oracle's PL/SQL library: DBMS_LOCK.SLEEP() .

SLEEP() is a simple program, which suspends processing for a parameterized number of seconds. This is not something we want to do often, but it is useful in testing. The problem is its home in the DBMS_LOCK package, because that package is not granted to public by default.

DBMS_LOCK is a utility package for building our own locking mechanisms. There's not much need for this any more. Oracle's default locking model is pretty good. There is SELECT .. FOR UPDATE for pessimistic locking, which is even more powerful since the SKIP LOCKED syntax was permitted in 11g. We have Advanced Queuing, Job Scheduling, oh my. It's hard to find a use case for user-defined locks which isn't re-inventing the wheel, and easy to see how we might end up implementing something less robust than the built-in locks. So DBAs tend not to grant execute on DBMS_LOCK without being asked, and then often not without a fight.

But as developers we need access to a sleep routine. So DBAs have to grant execute on DBMS_LOCK, and then that gives away too much access. It would be better if SLEEP() was easily accessible in some less controversial place.

Why is this an example of bad design? Because user-defined locks need a sleep routine but  SLEEP()has other uses besides lock implementations. Putting  SLEEP() in DBMS_LOCK means it's harder to use it.

Riding the Hobby Horse


Occasionally in a recruitment interview I have asked the candidate how they go would design a PL/SQL program. Mostly the question is met with bemusement. PL/SQL design is not A Thing. Yet many of us work on huge PL/SQL code-bases. How do they turn out without a design methodology? Badly:
  • Do you have one schema crammed with hundreds of PL/SQL program units, perhaps named with a prefix to identify sub-systems?
  • Do you have a package called UTILS?
  • Do you query USER_PROCEDURES or USER_DEPENDENCIES (or even USER_SOURCE) to find a piece of code which implements some piece of functionality?
  • Do you have the same functionality implemented in several places?
  • Does a "simple change" cascade into changes across multiple program units and a regression testing nightmare?
All these are symptoms of poor design. But there are ways to avoid this situation.

Designing PL/SQL Programs series

3 comments:

Colin 't Hart said...

Most DBAs don't realize they can grant execute on dbms_lock to eg "sleep_owner" and create a procedure in sleep_owner called sleep which simply invokes dbms_lock.sleep Finally, grant execute on sleep_owner.sleep to public (or whoever needs it).

Jeffrey Kemp said...

@Colin: a good example of adding a wrapper to work around a poor design.

Obviously the issue here is that the existing code can't simply be changed - first Oracle needs to provide a suitable alternative, deprecate dbms_lock.sleep for one or two releases, giving everyone enough time to migrate their code across. I'd say there's a huge amount of code out there that refers to dbms_lock.sleep so this won't be trivial.

The point of course is not to worry too much about dbms_lock.sleep, it's an example of a common problem where APIs are structured with (perhaps) not enough foresight. I can imagine the original developer of dbms_lock needed a "sleep" routine internally, and then someone asked "can we use that too" and he/she just added it to the package spec, instead of moving it out to another package.

William Robertson said...

Oh, I think they realise it. It's just that the sheer amount of paperwork, meetings and business sign-off needed to get a new schema deployed to production often isn't worth it just for one measly sleep() function.