I don’t have time for this

“I am a DBA, I am busy, too busy. Developers keep pushing changes to production without me reviewing the code, and now I am stuck again, over the weekend fixing performance issues while the developers are on the beach with a pina colada and a cigar” Sound familiar? Maybe the developers aren’t on the beach drinking and smoking, but the sentiment is the same:

• You are busy, you need to ensure your backups are restorable, your servers are locked down, new instances need provisioning, patching doesn’t stop, and those maintenance plans won’t maintain themselves (well olla’s probably will)
• Developers job is to, well, develop. They need to make changes, but the typical ratio of DBA’s to developers isn’t favourable. Typically there are more developers churning out more code than a DBA can manage
• Because the DBA’s don’t have time to review everything, changes go to production, cause issues which the DBA’s have to look at. If the DBA’s had the time to look at stuff, then they would have looked at the code before it was reviewed

It is a nightmare, what could there be?

Imagine if you have an army of workers who could review the code for you, what if you had one code reviewing soldier, who you trusted, for every developer? What if you could, over time, help the developers gain enough skills where they were capable of reviewing their own SQL code - what if that cut down on the amount of extra time you had to spend fixing things by 1%, 10%, 50%, 100%? Can you see yourself on that beach with your own pina colada and cigar?

Sold, but how? My developers don’t know SQL

OK so if we break this down into what a DBA should be doing as part of a code review:

• Ensure formatting is correct and any standards followed
• Have they introduces a SQL injection vulnerability?
• Consider any side effects of the actual change, for instance altering a clustered key on a 1 billion row table will take time - is this possible on a live system?
• Consider any performance effects - is this more prone to tempdb spills? How about deadlocks? Is the plan going to be terrible?
• Is the code going to do what the developer wants? Do they have the update statement correct in the merge statement?

That’s a lot, how can we help developers understand enough so that they can review their own code and cause fewer issues in production?

It is a lot, and that is part of the problem, quickly scanning some code is easy but doesn’t do much - spending time thinking about a change takes time and often finds issues, but the DBA team don’t have the time. This is what I like to call a vicious circle.

Let’s break down each thing that a DBA does and talk about what we can do about it:

Ensure formatting is correct, and any standards followed

Automate this 100%, on check-in run something to validate that this is correct, write your own tool to do it - use the scriptdom that is part of the DacFx or buy Redgate SQL Code Guard or some other means. Even better is to automatically format the code on check-in if it doesn’t meet your standards this way you know if the code is checked-in, it is formatted.

NOTE: I assume your code is in source control if it isn’t stop everything and do that (maybe don’t stop the backups)

Have they introduced a SQL injection vulnerability?

Automate this 100%, are “risky” process like sp_executesql or EXEC being used? You can fire a warning if there are or you can examine the code to see if any strings are concatenated and passed to the proc.

Consider any side effects of the actual change / consider any performance effects

I have merged these two together, there are two separate things you can do here, the first is to provide a good test environment where developers can write performance tests. These tests can be run on every check-in and if critical queries (include deployment scripts) don’t complete within a specified time, fail the build.

The second thing you can do is to work with your developers to train them, things like 1-hour lunchtime sessions are a great way to start sharing information. Topics for SQL Server include “how to read execution plan”, “SQL Server and indexes”, “Statistics for the inquisitive” and I am sure you can come up with lots more yourselves. I find these really work when both developers and DBA’s alike write and give presentations the developer ones, will likely, be less in-depth but that is fine, the developers will learn something and maybe so will the DBA.

Once you have some good performance tests, which someone will need to write and the developers have learnt how to do things like look at execution plans, then life suddenly becomes much more relaxed. If you foster an environment of learning, give guidance when needed then you can start to leave the code reviews to the developers who can deal with the day to day work and get help from the newly freed up DBA team on the more exciting parts of the database.

Is the code going to do what the developer wants?

This is a really key one for me, you can buy as many formatting tools as you like, but even the most beautiful code in the world can still have massive logic bugs in it. When that code is T-SQL, we can use tSQLt, the testing framework, to make sure that the code we write meets a set of defined criteria. Code reviews can look at the tests that have been written and the requirements and see if the conditions have been met with the tests.

Honestly, once you start using unit tests for your database code, and you use processes like code reviews to monitor the quality of the work, combine that with creating a culture of learning and sharing, and you can get back some of the wasted time. Just think, you and the developers will be sitting on that beach drinking the pina colada and smoking those cigars (note there are other ways to spend your time other than drinking, if you free up some of your work time then maybe you could even take up a new hobby!)