Since I've been doing a bit of testing work lately, I decided to spend some of my time with the SQLServerCentral databases. One of the first things I did was add the tsqlt framework to the database using SQL Test. This automatically installed the SQL Cop tests, which I then executed. The results were interesting, and I wanted to examine them from the perspective of a DBA on a real world application.

Executing SQL Cop Tests

I used SQL Test in my work since it's made my by employer, Red Gate. For me, running the SQL Cop tests was simple. I right clicked the class, and I selected "Run Tests".

This is the same as executing this code in a query window. Both of these use the tsqlt framework to execute all tests contained in a test class.

EXEC tSQLt.RunTestClass 'SQLCop'

There are a lot of SQL Cop tests, if we look at the list. SQL Test installs 47 tests in the 1.5 version that I was using.

The results were interesting as well. I wasn't sure how many would trip on the production database, but it turns out there were 18 items which failed.

I wasn't overly worried, as the application has run fine, but I did find this interesting. In terms of the tests, I think some of these are good development tests, and some are more production level tests for things like Auto Shrink. However, since tsqlt isn't built for production work, I don't love the idea of some of these administrative tests being run this way. I need to remove some of these as they're not applicable for development work, IMHO.

The tests that failed for me were:

• Columns of data type Text/nText
• Columns with float data type
• Columns with image data type
• Database and Log files on the same disk
• Instant File Initialization
• Max degree of parallelism
• Missing Foreign Key Indexes
• Missing Foreign Keys
• Old Backups
• Procedures that call undocumented procedures
• Procedures With SET ROWCOUNT
• Procedures without SET NOCOUNT ON
• SMO and DMO
• Tables without a primary key
• Tables without any data
• Unnamed Constraints
• Wide Table
• xp_cmdshell is enabled

I wanted to resolve all of these tests, one way or the other, so I'll discuss each of the failures below, along with the mitigation that I decided upon. I am including a few tests in the sections below that passed, but I decided to remove.

Note: I'm not necessarily recommending the same settings for your development (or other) environments. These are the choices I made for this application, and for this environment. I'll try to explain my choices as best I can.

Columns of data type Text/nText

The text/ntext datatypes are deprecated. As we run SQL Server 2008, and may upgrade to a version that doesn't support these, we shouldn't be using them. However I assume these are legacy holdovers from our previous instance, which was SQL Server 2000. All of SQLServerCentral was originally built on SQL Server 2000, and even though Red Gate developers rewrote much of the database, I suspect a few items carried over.

I can query the database with this code to find these columns:

 SELECT 
        o.name
    ,   c.name
    FROM
        sys.columns AS c
        INNER JOIN sys.objects AS o
        ON o.object_id = c.object_id
        INNER JOIN sys.types AS t
        ON t.system_type_id = c.system_type_id
    WHERE
        t.name IN ('ntext', 'text');

When I do this, I find that there are quite a few tables with text, but the Scripts table is the only offender, with two columns, for type ntext. This provides me with a bit of history, as I can see that the various tables that deal with emails have the text datatype. Not surprising as those were developed in conjunction with the mass email process by us Americans. We don't usually think of ntext.

However there are a few tables, like the Questions table and the Users table,  which were built by Red Gate developers with text types. The Scripts table, I'm not surprised, uses ntext, which makes sense. We do get scripts sometimes submitted with other languages embedded in them.

The fix for this is to alter these columns to the varchar(max) type. It's not a complex change, and according to MVP Michael Swart, this is a metadata change. However there is still a possibility of blocking on the production database, so we'll want to schedule these during a low activity time. Here would be the script:

ALTER TABLE dbo.Questions ALTER COLUMN Explanation NVARCHAR(max);
ALTER TABLE dbo.EmailsArchive ALTER COLUMN BodyHTML NVARCHAR(max);
ALTER TABLE dbo.EmailsArchive ALTER COLUMN BodyPlainText NVARCHAR(max);
ALTER TABLE dbo.Emails ALTER COLUMN BodyHTML NVARCHAR(max);
ALTER TABLE dbo.Emails ALTER COLUMN BodyPlainText NVARCHAR(max);
ALTER TABLE dbo.Adverts ALTER COLUMN PlainTextRepresentation NVARCHAR(max);
ALTER TABLE dbo.Scripts ALTER COLUMN SqlCode NVARCHAR(max);
ALTER TABLE dbo.Scripts ALTER COLUMN Rgtool NVARCHAR(max);
ALTER TABLE dbo.Users ALTER COLUMN Biography NVARCHAR(max);
ALTER TABLE dbo.ContentItems ALTER COLUMN text NVARCHAR(max);

Columns with float data type

This test surprised me, as I hadn't consider float to be an issue, but when I read the explanation page, it makes sense. We don't want real or float when we are trying to do math. Pinal Dave also has a good explanation.

How do we fix this? If you work in software development you know that this is the type of fix that can cause headaches. While it should be a simple change, we really need to do some work and identify how the columns are used and what precision and scale are really needed. There are ten columns that use this, and I'll have to go investigate. Hold on...

I'm back. 

Columns with image data type

This is the same issue as the text/ntext issue above. A simple fix here:

ALTER TABLE dbo. FileBlobs ALTER COLUMN Data varbinary(max);

Database and Log files on the same disk

This is one of those administrative checks that I don't think makes sense in SQL Cop. I'd use Policy Based Management to detect this in production, as I wouldn't have SQL Cop there. In that case, if I care about this in development, I'd use PBM here as well. This isn't a good idea, but I'm not sure I want developers concerned about this.

I'm deleting this test.

Instant File Initialization

This is another administrative item. Not worth worrying about on development. 

Deleting the test.

Max degree of parallelism

This is another administrative item. Not worth worrying about on development. 

Deleting the test.

Missing Foreign Key Indexes

This item is covered in a short blog (with a lot of code) from Jason Strate. Adding the indexes can give performance increases, and in this case, as this is based on the UserPoints table, I'd like an index here. We don't use this a lot, but we do use it intensively at times, so having this in place could speed up the queries.

The fix:

CREATE NONCLUSTERED INDEX [NCI_UserLogins_User] ON [dbo].[UserLogins]
(
[UserID] ASC
); 

The interesting thing after running this code is that the test still fails. I've got multiple items returned here but they aren't all returned by the test. The solution for me is to run the query from Jason's blog, and I find I have 29 of these. I'll have to go through, generate all the scripts, and then send them to our IT team for testing.

The fix is actually 30 DDL statements like the one above.

Missing Foreign Keys

I am a big fan of ensuring you have DRI setup in your database. Especially as we grow the number of applications that might access data, we want to be sure that DRI is in place. This test checks to determine if you are potentially missing FKs but comparing column names. In my case, I find 7 tables and 22 columns to check. I need to investigate.

Old Backups

This is certainly an administrative check, but since developers can create databases and might forget backups, this one might not be too bad. In my case, I created a backup process, but on this machine, the SQL Agent got stopped and didn't restart. As a result, my backups are more than 7 days old.

I'm leaving this, and starting SQL Agent. 😉

Procedures that call undocumented procedures

This looks like a false positive. While you want to be way of calling undocumented procedures, this references a user and procedure that shouldn't be in my database. I think my test system has issues for some reason. Perhaps during testing I ended up with a user and procedure that aren't needed. I've removed them from the test system as they aren't in the production system.

Procedures With SET ROWCOUNT

There are two procedures in my system that use SET ROWCOUNT inside of them. This is one of the deprecated T-SQL items, which has been listed in BOL for a few versions. I don't expect this to actually disappear anytime soon since lots of legacy code out there uses SET ROWCOUNT. I also don't expect us to upgrade anytime soon, given the way we use SQL Server and the high licensing costs.

I've made a note for developers to changes this to TOP at some point, but since this works, and it's low priority, I don't expect this to happen soon. This is a perfect place to refactor code while other development is ongoing, and make these changes now. That's what I recommend.

Procedures without SET NOCOUNT ON

We want SET NOCOUNT ON in our procedures to prevent unnecessary, and potentially very chatty, batches from sending messages back to the client. Potentially some clients might not like these messages, but they are unlikely to cause issues. The recommendation is that we use SET NOTCOUNT ON as the first line in our stored procedures.

There are a couple dozen procedures without this setting in them, and I'll ask that someone slowly fix them over time. This isn't really priority work, so I don't expect it to be done anytime soon.

SMO and DMO

Distributed Management Objects (DMO) have been deprecated and SQL Server Management Objects (SMO) are preferrred. I have these enabled on a development system, mostly because I do test things. On the production system, these are set at the default, which is enabled. Being enabled means that these objects can be used with SQL Server.

This really is more of an administrative check, not a development one. I am deleting this check and using PBM to manage this in production.

Tables without a primary key

This is a no-no, IMHO. Every table should really have a Primary Key (PK), unless there is a really good reason why not. I rarely run into good reasons, and none of the tables without one in this case, have a good reason.

There are 7 tables without PKs, and without any indexes. Each of these has a surrogate, or good candidate for a PK, and the indexing would help with performance, even if these are rarely used tables. I'll add indexes to these tables in production, testing each one to be sure it doesn't detract from performance by comparing the before and after metrics from SQL Monitor.

Tables without any data

Tables that don't contain data aren't necessarily extraneous. It's possible in a development system that you haven't populated the tables yet, or it's possible these are staging type tables that are cleared. However this is a red flag to watch out for. Perhaps you have old objects that aren't needed.

In my case, this is a test system and I have a few tables I haven't generated any data for. All of these tables have data in production, so no changes here.

Unnamed Constraints

If you don't name your constraints, SQL Server will come up with some random name. In my case, I have [dbo.PK__AnalysisImageLog__1CBC4616], which isn't a good name. If there's an issue and I'm troubleshooting. I want something that makes more sense than this.

Lots of experienced developers recommend using sensible names, and I'm one of them. I don't care what the name is, or what your standard is, but use something that makes sense.

Wide Table

There is one table that fails this test in the SSC database. It's the Scripts table, and it's not wide. Three columns, two of which are nText (another issue). The page for SQL Cop notes this is designed to catch tables that define more than 8060 bytes as a possible row. For the columns I have, their max size is reported as 8000 bytes, so two of them exceed the limit.

Not a concern for us, but certainly something to watch out for. Developers commonly will build tables that can cause issues here, putting in multiple 8k varchar columns when they don't want to run out of space. Use the MAX types instead, but more importantly, think about a vertical partition of the table with these seldom used, but wide, columns.

xp_cmdshell is enabled

We have this enabled. I don't know why, and it hasn't been an issue. However, more importantly, this isn't a test I want to have developers running. I'm deleting this one.

Other Tests

There are other tests that I don't think I want in development. These are the ones I feel are irrelelvent for our environment, and I am deleting from the system.

• Agent service
• Auto close
• Auto create statistics
• Auto Shrink
• Auto Update Statistics
• Buffer Cache Hit Ratio
• Database Collation
• Database Mail
• Fragmented Indexes
• Login Language
• Orphaned Users
• Page Life Expectancy
• Service Accounts

Conclusion

The SQL Cop tests are a good way to get started in testing your code with the tSQLt framework. As I mentioned, I don't think all of them are approproate for development environments, but many of them are. The tests are fairly easy to write as well, and I could see many developers (including us) extending the series of tests for their specific needs.

I would like to see the project expand, and if you have a series of specific tests that you'd like to include, contact the project and submit your own tests. If you'd like to donate to contiue their work, you can do that on the project page as well.

SQL Cop