Tuesday, June 12, 2007

Common MS SQL “Mistakes” - Part 1

In the last year or so I have moved more in to more consulting and contracting based work which means work working with a larger spread of developers with a an equally wide range of skills and abilities. There have been many generic mistakes I have scene (as well as some great code) but the majority of major mess ups I have seen is by far based around Database and Data Access. This is probably one of the major reasons why so many people are moving to ORM’s, because it removes the responsibility of creating the DA layer and the potential pitfalls that can come with it. This is fine, but it also means that a lot of people are becoming ignorant of the details of these layers. Generally these layers are the very foundations on which the application or applications are built on, I believe the blasé nature in which the technologies are approached is the reason why some run in to such major issues mid project. This late in the scene it is very hard to go back and make schema changes, especially if multiple teams are working off this schema.

I have decided to compile a list of issues that I have run in to in the last year, that I believe are potential short comings of the data related architecture. Often these points by themselves are not huge issues, but when I see one I usually can find another just around the corner.

Hopefully these come as some help to you and your design. Please remember this is coming from a developers perspective, a .Net and MS SQL developer at that, and I, in no way, proclaim myself to be a guru in any regard, however I feel these are basic points should be a general rule of thumb.

Database structure
Normalisation
Normalisation to the 3rd normal is standard. If you don’t know what 3rd normal is then it is a good hint that maybe you may not be the best one to be designing the DB. Sure there are reasons to move away from this rule but it should be just that, a reason, not bad design. Perhaps you may even want to document these reasons so it is clear why.
Nullable Columns
There will always be a need for nullable columns, however when tables start to have a growing number of nullable columns, you should start questioning why. Perhaps these can be moved to a separate table.

Property Tables
If a table is starting to have a lot of columns and many of them are null then you may want to implement a Property table and a PropertyType table. This is relatively clean and easy to set up and if done properly, is a fast way of getting a potentially large albeit often unknown number of properties, and a lot cleaner.

Incorrect Use of the Primary Key
Recently I saw a table that had a multi column primary key over five columns. After spanning more than 2 columns alarm bells go off in my head. In this case this was just bad design. The table only needed one Primary Key with one index on another column. Often this set up I find is good especially when there are unique values such as “Product Codes” that could be the PK but are in a format that does not suit performance (e.g. varchar(1000)). Using a smaller data type for the primary key improves performance. I also generally prefer to use [int] IDENTITY(1,1) as the default primary key for tables. Using large columns as PK’s in my mind is a bit of a no-no. Some times GUID’s are used if there is movement across to other databases. If this is happening then there is probably a properly qualified DBA involved leaving us developers to move back up a layer J

Incorrect Use of the Foreign Key
I have actually seen foreign keys been “used” without the being specifically being set, meaning referential integrity can not be there! Why this was done, god only knows, as it was not documented.

Not Appling Basic Coding Standards
Commenting code is a given. I can’t stand working on projects where Classes, Methods, Properties or Parameter are not obviously named and well commented. Most developers share these views, especially one who have come into projects late. For some reason these principles are ignored when it comes to Database development. Columns get named things like “DscInd” and have no comments associated to them. As far as I am aware there are no real penalties for naming you columns something that is easily understandable. The benefits are however large. People now understanding what is actually being stored! These basic procedures also help spot flaws in any business logic. If the intention of a piece of code is explicitly commented then, if for some reason it is wrong, alarm bells go off a little easier.
Naming conventions are typically applied in managed code but again are often missed in DB Development. I could go on, but you get the general idea. Treat DB dev in the same way you would when it comes to basic procedure such as naming, commenting and using the correct types.

2 comments:

Lee Campbell said...

Hey mate, some Sql to allow commenting your Tables and Columns easier. Copy the output as text, and just edit the end string on each line.

/*
This script generates code to set the extended properties for all tables and their columns for the database.
Author Lee Campbell 20-May-2007
*/
SET NOCOUNT ON
DECLARE @output varchar(8000)
DECLARE @schema varchar(128), @tableId int, @table nvarchar(128), @columnId int, @column varchar(128), @extProperty varchar(7500)

PRINT '-------- Table extended properties --------'

DECLARE table_cursor CURSOR FOR
SELECT
t.object_Id as TableId,
s.name [schema],
t.Name [table],
Cast(value as varchar(7500)) as extProperty
from
sys.Tables t
INNER JOIN sys.Schemas s
on t.schema_id = s.schema_id
LEFT OUTER JOIN sys.Extended_properties p
on t.object_Id = p.major_id
and p.minor_id = 0
and p.name = 'MS_Description'
ORDER BY
s.name,
t.Name
OPEN table_cursor

FETCH NEXT FROM table_cursor
INTO @tableId, @schema, @table, @extProperty

WHILE @@FETCH_STATUS = 0
BEGIN
PRINT ' '
PRINT '--Table: ' + @schema + '.' + @table
If @extProperty IS NULL
BEGIN
SET @output = 'EXEC sp_addextendedproperty '
END
ELSE
BEGIN
SET @output = 'EXEC sp_updateextendedproperty '
END
PRINT @output + '@level0type = N''Schema'', @level0name = ''' + @schema + ''', @level1type = N''Table'', @level1name = ''' + @table + ''', @name = N''MS_Description'', @value = ''' + IsNull(Replace(@extProperty, '''', ''''''), '') + ''';'
PRINT 'GO'
PRINT '--Columns for ' + @schema + '.' + @table

DECLARE column_cursor CURSOR FOR
SELECT
c.column_id,
c.name,
Cast(value as varchar(7500)) as extProperty
from
sys.columns c
LEFT OUTER JOIN sys.Extended_properties p
on c.object_Id = p.major_id
and c.column_id = p.minor_id
and p.name = 'MS_Description'
WHERE
c.object_id = @tableId
ORDER BY
c.column_id
OPEN column_cursor

FETCH NEXT FROM column_cursor
INTO @columnId, @column, @extProperty
WHILE @@FETCH_STATUS = 0
BEGIN
If @extProperty IS NULL
BEGIN
SET @output = 'EXEC sp_addextendedproperty '
END
ELSE
BEGIN
SET @output = 'EXEC sp_updateextendedproperty '
END
PRINT @output + '@level0type = N''Schema'', @level0name = ''' + @schema + ''', @level1type = N''Table'', @level1name = ''' + @table + ''', @level2type = N''Column'', @level2name = ''' + @column + ''', @name = N''MS_Description'', @value = ''' + IsNull(Replace(@extProperty, '''', ''''''), '') + ''';'
PRINT 'GO'


FETCH NEXT FROM column_cursor
INTO @columnId, @column, @extProperty
END
CLOSE column_cursor
DEALLOCATE column_cursor
PRINT '--------------------------------------'
-- Get the next vendor.
FETCH NEXT FROM table_cursor
INTO @tableId, @schema, @table, @extProperty
END
CLOSE table_cursor
DEALLOCATE table_cursor

Lee Campbell said...

Hey mate, some Sql to allow commenting your Procs and paramters easier. Copy the output as text, and just edit the end string on each line. Any good code generator should take these comments and add them to the internal gateway call to the database.

/*
This script generates code to set the extended properties for all procedures and their paramters for the database.
Author Lee Campbell 20-May-2007
*/
SET NOCOUNT ON
DECLARE @output varchar(8000)
DECLARE @schema varchar(128), @procedureId int, @procedure nvarchar(128), @parameterId int, @parameter varchar(128), @extProperty varchar(7500)

PRINT '-------- Procedure extended properties --------'

DECLARE procedure_cursor CURSOR FOR
SELECT
sp.object_Id as procedureId,
s.name [schema],
sp.Name [procedure],
Cast(value as varchar(7500)) as extProperty
from
sys.procedures sp
INNER JOIN sys.Schemas s
on sp.schema_id = s.schema_id
LEFT OUTER JOIN sys.Extended_properties p
on sp.object_Id = p.major_id
and p.minor_id = 0
and p.name = 'MS_Description'
ORDER BY
s.name,
sp.Name
OPEN procedure_cursor

FETCH NEXT FROM procedure_cursor
INTO @procedureId, @schema, @procedure, @extProperty

WHILE @@FETCH_STATUS = 0
BEGIN
PRINT ' '
PRINT '--Procedure: ' + @schema + '.' + @procedure
If @extProperty IS NULL
BEGIN
SET @output = 'EXEC sp_addextendedproperty '
END
ELSE
BEGIN
SET @output = 'EXEC sp_updateextendedproperty '
END
PRINT @output + '@level0type = N''Schema'', @level0name = ''' + @schema + ''', @level1type = N''Procedure'', @level1name = ''' + @procedure + ''', @name = N''MS_Description'', @value = ''' + IsNull(Replace(@extProperty, '''', ''''''), '') + ''';'
PRINT 'GO'
PRINT '--Parameters for ' + @schema + '.' + @procedure

DECLARE parameter_cursor CURSOR FOR
SELECT
pr.parameter_id,
pr.name,
Cast(value as varchar(7500)) as extProperty
from
sys.parameters pr
LEFT OUTER JOIN sys.Extended_properties p
on pr.object_Id = p.major_id
and pr.parameter_id = p.minor_id
and p.name = 'MS_Description'
WHERE
pr.object_id = @procedureId
ORDER BY
pr.parameter_id
OPEN parameter_cursor

FETCH NEXT FROM parameter_cursor
INTO @parameterId, @parameter, @extProperty
WHILE @@FETCH_STATUS = 0
BEGIN
If @extProperty IS NULL
BEGIN
SET @output = 'EXEC sp_addextendedproperty '
END
ELSE
BEGIN
SET @output = 'EXEC sp_updateextendedproperty '
END
PRINT @output + '@level0type = N''Schema'', @level0name = ''' + @schema + ''', @level1type = N''Procedure'', @level1name = ''' + @procedure + ''', @level2type = N''Parameter'', @level2name = ''' + @parameter + ''', @name = N''MS_Description'', @value = ''' + IsNull(Replace(@extProperty, '''', ''''''), '') + ''';'
PRINT 'GO'


FETCH NEXT FROM parameter_cursor
INTO @parameterId, @parameter, @extProperty
END
CLOSE parameter_cursor
DEALLOCATE parameter_cursor
PRINT '--------------------------------------'
-- Get the next vendor.
FETCH NEXT FROM procedure_cursor
INTO @procedureId, @schema, @procedure, @extProperty
END
CLOSE procedure_cursor
DEALLOCATE procedure_cursor