This blog will no longer be updated.

New content is available on my new blog

Good practices - database programming, unit testing - Piotr Rodak

Good practices - database programming, unit testing

Jason Brimhall wrote today on his blog that new book, Defensive Database Programming, written by Alex Kuznetsov (blog) is coming to bookstores. Alex writes about various techniques that make your code safer to run. SQL injection is not the only one vulnerability the code may be exposed to. Some other include inconsistent search patterns, unsupported character sets, locale settings, issues that may occur during high concurrency conditions, logic that breaks when certain conditions are not met. The book covers these issues and provides examples how to write code to diminish chances of failure. Another good thing is that it is free, and you can download it from RedGate’s page.

Database code is often written ad hoc, by programmers who do not necessarily have in-depth knowledge of the database engine. There is poor error handling, careless usage of data types, resulting in implicit conversions at run time, cursors and loops and many other flavours. Apart from this, the code is very often tested only in a very rudimentary way, for a few (at most) parameter combinations and developers rarely take edge conditions into account.

It is quite easy to improve stored procedure coding standards by following just a few simple ideas. I find these most important:

  • Parameter validation – ensure that your procedure or batch operates on parameters within expected ranges. If certain value must not be null or empty string, check for it before you apply it into a query, you’ll save some resources. Of course this doesn’t mean that you shouldn’t have proper check constraints on your columns.
  • Error handling – along with the parameter validation it is a good practice to throw an error if some validation fails. If you write a procedure that adds new contact to database, it doesn’t make sense if its last name is empty. If you throw an error saying ‘Contact’s name must not be empty’, it will be much easier to debug and fix the problem if it occurs.
  • Testing – write unit tests that will ensure that your code works. Since there is no reasonable and easy way to write tests in T-SQL, you can use higher level framework, like NUnit for .NET. I find it sufficient in many cases to write a few tests straight in the T-SQL, wrapped in begin tran .. rollback blocks. These tests verify that the procedure works in expected way for parameters. These tests do not mimic exactly unit testing frameworks and do not have the Assert functionality. They let you though see at first glance if your code works for given set of parameters or not.
  • Proper naming of constraints and indexes – it helps a lot if you can associate name of the constraint with particular table without having to shuffle through sys.objects view.

Let’s have a look at little example.

Let’s assume that we have table that defines some feeds we handle in the system. Feeds can have lots of properties, but for our example I will use only FeedName, FeedCategory which describes category of feed and FeedType, which defines how the feed is processed.

First approach to the code as follows:

   1: create table FeedCategory
   2: (
   3:     CategoryId int identity(1, 1) not null primary key,
   4:     CategoryName varchar(128)
   5: )
   6: go
   7: create table Feed
   8: (
   9:     FeedId int identity(1, 1) not null primary key,
  10:     FeedName varchar(128) not null check (len(FeedName) > 0),
  11:     FeedCategory int not null references FeedCategory(CategoryId),
  12:     FeedType char(2)
  13: )
  14: go
  15: create procedure AddNewFeed
  16: (
  17:     @FeedName varchar(128),
  18:     @FeedCategoryName varchar(128),
  19:     @FeedType char(2)
  20: )
  21: as
  22: begin
  23:     declare @CategoryId int
  24:     select @CategoryId = CategoryId from FeedCategory where CategoryName = @FeedCategoryName
  25:  
  26:     insert Feed(FeedName, FeedCategory, FeedType)
  27:     values (@FeedName, @CategoryId, @FeedType)
  28: end
  29:  
  30: go

Now it' looks kind of OK, let’s write a test.

First test that I will write is to check what happens if the FeedName is invalid:

   1: begin tran
   2:     raiserror('Add new feed, empty feed name', 10, 1) with nowait;
   3:     insert FeedCategory (CategoryName) values('Maintenance feeds')
   4:     exec AddNewFeed @FeedName = '', @FeedCategoryName = 'Maintenance feeds', @FeedType='FS'
   5: rollback
   6: go

As you see, tests are built in simple way. All test code is wrapped in begin tran – rollback block. First line after the beginning of transaction outputs test description, so you can see which test fails or succeeds. If you run the above test, an error message is returned:

 

   1: Add new feed, empty feed name
   2:  
   3: (1 row(s) affected)
   4: Msg 547, Level 16, State 0, Procedure AddNewFeed, Line 12
   5: The INSERT statement conflicted with the CHECK constraint "CK__Feed__FeedName__2F650636". The conflict occurred in database "testdb", table "dbo.Feed", column 'FeedName'.
   6: The statement has been terminated.

I don’t like this error message. Look how ugly the name of the CHECK constraint is. Let’s have a look what can be done about this.

If you change the name of the constraint to something more meaningful, like in the following code,

   1: create table Feed
   2: (
   3:     FeedId int identity(1, 1) not null primary key,
   4:     FeedName varchar(128) not null,
   5:     FeedCategory int not null references FeedCategory(CategoryId),
   6:     FeedType char(2),
   7:     constraint CHK_Feed_FeedNameNotEmpty check (len(FeedName) > 0)
   8: )

you will get better error message:

   1: Add new feed, empty feed name
   2:  
   3: (1 row(s) affected)
   4: Msg 547, Level 16, State 0, Procedure AddNewFeed, Line 12
   5: The INSERT statement conflicted with the CHECK constraint "CHK_Feed_FeedNameNotEmpty". The conflict occurred in database "testdb", table "dbo.Feed", column 'FeedName'.
   6: The statement has been terminated.

Well, this looks better, but in my opinion, it is still lacking elegance.

To fix this, you can modify the procedure:

   1: create procedure AddNewFeed
   2: (
   3:     @FeedName varchar(128),
   4:     @FeedCategoryName varchar(128),
   5:     @FeedType char(2)
   6: )
   7: as
   8: begin
   9:     begin try
  10:         if isnull(@FeedName, '') = ''
  11:             raiserror('Feed name must not be empty', 16, 1)
  12:  
  13:         declare @CategoryId int
  14:         select @CategoryId = CategoryId from FeedCategory where CategoryName = @FeedCategoryName
  15:  
  16:         insert Feed(FeedName, FeedCategory, FeedType)
  17:         values (@FeedName, @CategoryId, @FeedType)
  18:     end try
  19:     begin catch
  20:         declare @err varchar(2000)
  21:         set @err = 'Exception in '  + error_procedure() + ', error line '+convert(varchar, error_line())+': ' + error_message()
  22:         raiserror(@err, 16, 1)
  23:     end catch
  24: end
 

Now the error message looks like this:

   1: Add new feed, empty feed name
   2:  
   3: (1 row(s) affected)
   4: Msg 50000, Level 16, State 1, Procedure AddNewFeed, Line 23
   5: Exception in AddNewFeed, error line 12: Feed name must not be empty
Ok, this looks much better in my opinion. We can say that test 1 has passed by properly failing the transaction and returning user friendly message. If you want to mimic Assert functionality of NUnit you will have to slightly modify the test code:
 
   1: --assert like test
   2: begin tran
   3:     begin try
   4:         raiserror('-- ================= --', 10, 1) with nowait;
   5:         raiserror('Test 1: Add new feed, empty feed name', 10, 1) with nowait;
   6:         insert FeedCategory (CategoryName) values('Maintenance feeds')
   7:         exec AddNewFeed @FeedName = '', @FeedCategoryName = 'Maintenance feeds', @FeedType='FS'
   8:         raiserror('FAIL', 16, 1) --Assert.Fail. We shouldn't be here.
   9:     end try
  10:     begin catch
  11:         declare @err varchar(2000)
  12:         set @err = error_message()
  13:         if @err like '%Feed name must not be empty%' --is it expected exception?
  14:             raiserror('Test succeeded.', 10, 1) with nowait;
  15:         else
  16:             raiserror('Test failed: %s', 16, 1, @err);
  17:     end catch
  18: rollback
 
OK. So far, we have one test covered, that checks if the feed name is correct. As you noticed, the AddNewFeed procedure accepts also category name and performs lookup in the FeedCategory to retrieve the CategoryId. It is sometimes better to perform lookups, especially if the procedure is going to be called by support directly from management studio. This procedure is meant to be part of API to manage feeds, so it has to be user friendly to some extent. Fiddling with identifiers is not very convenient and error prone.
I will skip the discussion about naming of the foreign key constraint here, but let’s have a look at the code that handles failed lookups. If you don’t have these lines in the procedure, insert will fail because of foreign key constraint, but as before, more friendly message is going to be appreciated by your users.
   1: create procedure AddNewFeed
   2: (
   3:     @FeedName varchar(128),
   4:     @FeedCategoryName varchar(128),
   5:     @FeedType char(2)
   6: )
   7: as
   8: begin
   9:     begin try
  10:         if isnull(@FeedName, '') = ''
  11:             raiserror('Feed name must not be empty', 16, 1)
  12:  
  13:         declare @CategoryId int
  14:         select @CategoryId = CategoryId from FeedCategory where CategoryName = @FeedCategoryName
  15:  
  16:         if @CategoryId is null
  17:             raiserror('Invalid feed category name', 16, 1)
  18:         
  19:         insert Feed(FeedName, FeedCategory, FeedType)
  20:         values (@FeedName, @CategoryId, @FeedType)
  21:     end try
  22:     begin catch
  23:         declare @err varchar(2000)
  24:         set @err = 'Exception in '  + error_procedure() + ', error line '+convert(varchar, error_line())+': ' + error_message()
  25:         raiserror(@err, 16, 1)
  13:         if @err like '%Invalid feed category name%' --is it expected exception?
  26:     end catch
  27: end

Please notice lines 16 and 17 in this snippet. I detect the fact that particular category name could not be found in FeedCategory table and throw appropriate error.

One of aspects of unit testing is that each unit test should test only one condition. We could modify Test 1 to cover invalid category name situation, but we will write new test, so both cases are logically separated.

   1:  
   2: begin tran
   3:     begin try
   4:         raiserror('-- ================= --', 10, 1) with nowait;
   5:         raiserror('Test 2: Add new feed, invalid category name', 10, 1) with nowait;
   6:         insert FeedCategory (CategoryName) values('Maintenance feeds') --insert category
   7:         exec AddNewFeed @FeedName = 'Feed 1', @FeedCategoryName = 'No such category', @FeedType='FS'
   8:         raiserror('FAIL', 16, 1) --Assert.Fail. We shouldn't be here.
   9:     end try
  10:     begin catch
  11:         declare @err varchar(2000)
  12:         set @err = error_message()
  13:         if @err like '%Invalid feed category name%' --is it expected exception?
  14:             raiserror('Test succeeded.', 10, 1) with nowait;
  15:         else
  16:             raiserror('Test failed: %s', 16, 1, @err);
  17:     end catch
  18: rollback

As you see in the line 13, I changed the expected error message to test invalid category name. If this exception is thrown, test succeeds.

   1: -- ================= --
   2: Test 2: Add new feed, invalid category name
   3:  
   4: (1 row(s) affected)
   5: Test succeeded.
I will leave adding test for valid FeedType values to you, I hope that this post gives you an idea how easy it is to improve quality of code without spending too much time. These unit tests are pretty simple, but you can use them to perform basic validation of logic in your stored procedures. You are not likely to make your users smile, but your DBAs should be happier people if you follow these simple ideas.
 
 
 
 
 
Published 24 May 2010 01:27 by Piotr Rodak

Comments

# Database testing links « Michael Baylon's blog

Pingback from  Database testing links «  Michael Baylon's blog

# http://usepubs.blogspot.com/2010/06/coding-practices-call-your-constraints.html

14 June 2010 19:32 by TrackBack

# http://usepubs.blogspot.com/2010_05_30_archive.html

14 June 2010 19:32 by TrackBack