How PL/SQL Development Standards work

I have been gigging at a place which has documented PL/SQL Development Standards. This is not so unusual: most Oracle shops have such a document. What makes it unusual is that they enforce the standards. With code reviews. And I mean properly enforce: programs fail QA for egregious breaches of the standards or a sufficient accumulation of minor breaches. This is less common than it ought to be.

Many coders are sceptical about development standards; I have been in the past. Standards generally focus on things which are easy to standardise (indentation, case, naming conventions) rather functional correctness or design principles. They frequently codify arbitrary or outdated practices (mandating explicit cursors is a particular bugbear of mine). They either go into so much detail that they are unreadably long (and dull) or are so sketchy that they operate as easy-to-ignore guidelines. But I think many experienced developers' objections boil down to: I don't like being told how to write my code; my style is the best style; my code is clean, clear and readable.

The catch is, readability is not simply a function of personal style: it emerges from consistency across the entire codebase. Just because I find my personal coding style clear doesn't mean everbody else will. At the very least a colleague reading my program will have to invest time in understanding how I name my variables, how I use table aliases, and a dozen other things, none of them important individually but all together adding friction to the crucial task of understanding how a program works (or does not work).

This particular set of standards certainly had a lot to say about layout. Many strictures fitted with my natural coding style (all lower case, one column per line in a SQL projection, comma before the column name rather than after it). Others were rather tiresome: the rules for clause alignment entail a lot of spacing and backspacing to ensure elements line up. There are a few strictures I actively disagree with (notably mandatory use of SQL-89 syntax i.e. impicit joins). But here's the rub: I didn't get to pick and choose which of the standards I followed. I just had to knuckle down and follow them all. Because the discipline of the code review meant my programs failed QA when I hadn't applied the standards.

There's more to consistency than just layout and naming conventions. There's also functional consistency: use of SQL and PL/SQL idioms, how to organise programs within a package, and so forth. Too many things to cover in a single document. But again, code reviews enforce standardisation of these aspects, by applying undocumented conventions with the same rigour as documented standards. A couple of times I tripped over such an undocumented convention and it didn't feel fair: my code failed the review because I wrote something which was wrong even though not explicitly covered by the standards. One of these times it was something awry in the layout. "That's wrong", the reviewer said. It was a difference I hadn't even noticed, and probably you wouldn't have noticed either, and even if I had have noticed it I wouldn't have thought it was wrong. But it was different from what everybody else was doing. That made it wrong.

Everybody undertakes code reviews and everybody's code is reviewed. Thus code reviews shape the codebase, by enforcing documented standards and undocumented conventions. As a result this is the most readable codebase I have ever worked on. It's almost impossible to tell who wrote any given program, because all programs look the same. It's easy to reason about a piece of code because it follows rigorous naming conventions and consistent architectural principles. The code is habitable. A colleague can read a program I wrote and feel comfortable doing so. The layout, the naming conventions, the consistent selection of one approach in situations where PL/SQL offers more than one way of doing something, all these factors mean my program looks just like the program anybody else would have written. So the reader is freer to understand what the program actually does and how it works. Standardisation reduces friction.

It is a virtuous circle. Code reviews enforce a consistent programming style, which eliminates trivial (i.e. non-functional) differences in the program. In turn this makes the program easier to review: all the programs look basically the same which highlights the things which need to be different, the business logic and the data structures.

Readability is a feature

Readility is a feature. It's a feature our code must have. We all know readability makes code easier to maintain, easier to re-use, easier to debug. Yet still many developers bridle at the suggestion that their PL/SQL must look like everybody else's PL/SQL. I get this. It's not that I think the way I write PL/SQL is intrinsically correct, it just looks the way I have evolved to write it over the years. A new set of coding standards, rigorously applied, disrupts my flow. I must slow down to correct the variable names or fix the layout. It's tedious.

Tedious but also necessary. A sofware system is a shared enterprise. It's not "my" code, it's the project's code; I am just the person checking it into source control. As a discipline, programming is a craft not an art. PL/SQL is simply a device for turning data into business value. It's more important that other people on the team can work with our code than that it has our signature style. So let's not be precious about appearance. We must follow the rules, and save our self-expression for our poems and our tweets.

Above all, know this: there are no development standards without code reviews.

Let me SLEEP!

DBMS_LOCK is a slightly obscure built-in package. It provides components which so we build our own locking schemes. Its obscurity stems from the default access on the package, which is restricted to its owner SYS and the other power user accounts. Because implementing your own locking strategy is a good way to wreck a system, unless you really know what you're doing. Besides, Oracle's existing functionality is such that there is almost no need to need to build something extra (especially since 11g finally made the SELECT ... FOR UPDATE SKIP LOCKED syntax legal). So it's just fine that DBMS_LOCK is private to SYS. Except ...

... except that one of the sub-programs in the package is SLEEP(). And SLEEP() is highly useful. Most PL/SQL applications of any sophistication need the ability to pause processing for a short while, either a fixed time or perhaps polling for a specific event. So it is normal for PL/SQL applications to need access to DBMS_SLEEP.LOCK().

Commonly this access is granted at the package level, that is grant execute on dbms_lock to joe_dev. Truth to be told, there's not much harm in that. The privilege is granted to a named account, and if somebody uses the access to implement a roll-your-own locking strategy which brings Production to its knees, well, the DBAs know who to look for.

But we can employ a schema instead. The chief virtue of a schema is managing rights on objects. So let's create a schema for mediating access to powerful SYS privileges:

create user sys_utils identified by &pw
temporary tablespace temp
/
grant create procedure, create view, create type to sys_utils
/

Note that SYS_UTILS does not get the create session privilege. Hence nobody can connect to the account, a sensible precaution for a user with potentially damaging privileges. Why bar connection in all databases and not just Production? The lessons of history tell us that developers will assume they can do in Production anything they can do in Development, and write their code accordingly.

Anyway, as well as granting privileges, the DBA user will need to build SYS_UTIL's objects on its behalf:

grant execute on dbms_lock to sys_utils
/
create or replace procedure sys_utils.sleep
    ( i_seconds in number)
as
begin
    dbms_lock.sleep(i_seconds);
end sleep;
/
create public synonym sleep for sys_utils.sleep
/
grant execute on sys_utils.sleep to joe_dev
/

I think it's a good idea to be proactive about creating an account like this; granting it some obviously useful privileges before developers ask for them, simply because some developers won't ask. The forums occasionally throw up extremely expensive PL/SQL loops whose sole purpose is to burn CPU cycles or wacky DBMS_JOB routines which run every second. These WTFs have their genesis in ignorance of, or lack of access to, DBMS_LOCK.SLEEP().