SQLGeordie's Blog

Helping the SQL Server community……where i can!

Making sure your Triggers fire when they should — March 4, 2013

Making sure your Triggers fire when they should

As some of you may be aware, triggers are not my favourite thing in the world but like most things, it does have its place.

Whilst onsite with one of my clients, one of the processes fires a trigger on insert which ultimately runs a SSRS subscription to email a report. All sounding fairly feasible so far. However, this process is also used as part of a batch process overnight which would run a separate insert statement (actually another stored procedure in another job step) instead of the “onDemand” insert. Ok, still doesn’t sound like too much of an issue.

Now, they started experiencing occasional failures of this job during the day with the error relating to the fact that the SSRS subscription job was being called when it already was running. Interesting, this in theory shouldn’t ever happen because the process either ran the jobs based on the batch process or the one off onDemand.

Stepping through the process, it led me to an AFTER INSERT trigger. Upon opening it I spotted the issue straight away. Something that as I’ve found over the years as a consultant, a lot of DBA’s and developers have failed to understand that (from MSDN ):

These triggers fire when any valid event is fired, regardless of whether or not any table rows are affected.This is by design.

So, the issue was that step 3 ran a procedure which ultimately ran an insert statement for the onDemand insert, step 4 ran a procedure to insert for the overnight batch process which as it happens doesn’t have any records to insert but will in fact fire the trigger to run the SSRS subscription again! There is a number of ways to fix this but I’ve tended to stick with a basic check of the “inserted” table for results and RETURN out if no records are there to process.

I’ve supplied a bit of test code below for people to try this out.

Lets create a test table and an audit table:

USE tempdb
GO

IF  EXISTS (SELECT * FROM sys.objects WHERE object_id = OBJECT_ID(N'[dbo].[TestTable]') AND type in (N'U'))
DROP TABLE [dbo].[TestTable]
GO
CREATE TABLE [dbo].[TestTable]
(
	TestTableID INT IDENTITY(1,1),
	TestTableDescr VARCHAR(20)
)
GO

IF  EXISTS (SELECT * FROM sys.objects WHERE object_id = OBJECT_ID(N'[dbo].[AuditTrigger]') AND type in (N'U'))
DROP TABLE [dbo].[AuditTrigger]
GO
CREATE TABLE [dbo].[AuditTrigger]
(
	AuditTriggerID INT IDENTITY(1,1),
	AuditTriggerDescr VARCHAR(20),
	DateCreated DATETIME
)
GO

INSERT INTO dbo.TestTable (TestTableDescr)
VALUES ('Test1'), ('Test2'), ('Test3');

SELECT * FROM dbo.TestTable;

Now lets create the trigger with no checking:

USE [TempDB]
GO

IF  EXISTS (SELECT * FROM sys.triggers WHERE object_id = OBJECT_ID(N'[dbo].[trTestTable]'))
DROP TRIGGER [dbo].[trTestTable]
GO

CREATE TRIGGER [dbo].[trTestTable] ON [dbo].[TestTable]
   AFTER INSERT
AS
BEGIN

	--Log the fact the trigger fired
	INSERT INTO [dbo].[AuditTrigger] (AuditTriggerDescr, DateCreated)
	SELECT 'Trigger Fired', GETDATE()

END
GO

Test Inserting a record that exists:

--Valid Insert
INSERT INTO dbo.TestTable (TestTableDescr)
SELECT TestTableDescr
FROM dbo.TestTable
WHERE TestTableDescr = 'Test1';

SELECT  *
FROM    [dbo].[AuditTrigger];

Test Inserting a record that doesn’t exist:

--Not a Valid Insert
INSERT INTO dbo.TestTable (TestTableDescr)
SELECT TestTableDescr
FROM dbo.TestTable
WHERE TestTableDescr = 'Test4';

SELECT  *
FROM    [dbo].[AuditTrigger];

You’ll now see that there are 2 entries in the AuditTrigger table due to the fact that the trigger fired even though no records were actually valid to insert.

So, lets amend the trigger to check for valid inserts:

USE [TempDB]
GO

IF  EXISTS (SELECT * FROM sys.triggers WHERE object_id = OBJECT_ID(N'[dbo].[trTestTable]'))
DROP TRIGGER [dbo].[trTestTable]
GO

CREATE TRIGGER [dbo].[trTestTable] ON [dbo].[TestTable]
   AFTER INSERT
AS
BEGIN

	--Check to see if any records were inserted
	IF NOT EXISTS (SELECT 1 FROM INSERTED)
		RETURN 

	--Log the fact the trigger fired
	INSERT INTO [dbo].[AuditTrigger] (AuditTriggerDescr, DateCreated)
	SELECT 'Trigger Fired', GETDATE()

END
GO

and test the inserts again:

Test Inserting a record that exists:

--Valid Insert
INSERT INTO dbo.TestTable (TestTableDescr)
SELECT TestTableDescr
FROM dbo.TestTable
WHERE TestTableDescr = 'Test2';

SELECT  *
FROM    [dbo].[AuditTrigger];

Test Inserting a record that doesn’t exist

--Not a Valid Insert
INSERT INTO dbo.TestTable (TestTableDescr)
SELECT TestTableDescr
FROM dbo.TestTable
WHERE TestTableDescr = 'Test4';

SELECT  *
FROM    [dbo].[AuditTrigger];

No record will have been inserted with the final insert statement!

Lets clean up our tempdb:

USE [TempDB]
GO

--Clean up
IF  EXISTS (SELECT * FROM sys.objects WHERE object_id = OBJECT_ID(N'[dbo].[TestTable]') AND type in (N'U'))
DROP TABLE [dbo].[TestTable]
GO
IF  EXISTS (SELECT * FROM sys.objects WHERE object_id = OBJECT_ID(N'[dbo].[AuditTrigger]') AND type in (N'U'))
DROP TABLE [dbo].[AuditTrigger]
GO

Hopefully this will help point out the misconception that triggers only fire when records are actually inserted 🙂

As per usual, I’d like to hear peoples thoughts/experiences on this topic.

Advertisements
NetApp Dynamic Backup Script — June 9, 2011

NetApp Dynamic Backup Script

For those of you who’ve worked with NetApp and SnapManager for SQL Server, you will know can generate a scheduled backup as a SQL Agent Job. Simple, click through the GUI, selecting the Databases you want to backup, the schedule etc etc and a job is created with a command line script within it similar to that below:

"C:\Program Files\NetApp\SnapManager for SQL Server\SmsqlJobLauncher.exe" new-backup
–svr 'ServerToBackupName'
-d 'ServerToBackupName', '4', 'DBName1', 'DBName2', 'DBName3', 'DBName4'
-ver –verInst 'ServerToVerifyOnName' -mp
–mpdir 'C:\Program Files\NetApp\SnapManager for SQL Server\SnapMgrMountPoint'
-RetainBackups 7 -lb -bksif -RetainSnapofSnapInfo 0 -trlog –mgmt standard

This script indicates the server in which the Databases reside upon for backing up, the number of databases to be backed up as well the list of database names you selected earlier in the GUI.

Now, this looks all relatively straight forward and makes perfect sense up until the point when you either create or drop a database from the server. This script does not pick up the fact that this has happened and will fail to snap any new databases and will no doubt fail trying to snap a database that no longer exists. So, every time you do this administration work you have the additional step of amending the job to match the current list of databases as it cannot update itself – how crazy is that!?!

I’m not sure about you I don’t like giving myself additional work to do so I set about rectifying this issue by creating a script that would mean you can create this job once and not have to worry about it again unless you specifically require to exclude a certain database by dynamically populating the Database list for you based on what is in sys.databases. This can be modified to include or exclude certain Databases i.e. system databases.

The script itself is straightforward, assigning the list of Databases to a variable and build up a command string to be ran via xp_cmdshell. Note:- There are certain security risks associated with enabling the xp_cmdshell feature so please make sure you’ve read and understood this before proceeding .
I’ve added comments to the script where necessary:

SET NOCOUNT ON;

-- Amend variables where necessary depending on how many DBs you have
-- Remembering the maximum recommendation for DBs on a LUN (from NetApp) is 35
DECLARE 	@strSQL		        VARCHAR(2000), 
		@strDBNames		VARCHAR(1000),
		@intDBCnt		VARCHAR(5),
		@strVerifServer 	VARCHAR(50),
		@intRetainBkups 	INT

SELECT		@strSQL		        = '',
		@strDBNames		= '',
		@strVerifServer 	= 'VerificationServerName',
		@intRetainBkups 	= 8
		
--Get DB Names
SELECT @strDBNames = @strDBNames +''''+ NAME +''', '
FROM sys.databases
--WHERE database_id > 4

--Get DB count
SELECT @intDBCnt = CONVERT(VARCHAR(5),COUNT(*)) 
FROM sys.databases
--WHERE database_id > 4

--remove trailing comma, probably a fancier way but its quick and works
SET @strDBNames = LEFT(@strDBNames,LEN(@strDBNames)-1) 

--Make sure this string is all on one line!!!!
--Multi-line for readability
SET @strSQL = @strSQL + '
"C:\Program Files\NetApp\SnapManager for SQL Server\SmsqlJobLauncher.exe" new-backup  
–svr '''+@@SERVERNAME+'''
-d  '''+@@SERVERNAME+''', '''+@intDBCnt+''', '+@strDBNames+'
-ver  –verInst '''+@strVerifServer+''' -mp  
–mpdir ''C:\Program Files\NetApp\SnapManager for SQL Server\SnapMgrMountPoint''
-RetainBackups  '+CONVERT(VARCHAR(5),@intRetainBkups)+' -lb -bksif -RetainSnapofSnapInfo 0 -trlog  –mgmt standard 
'

SELECT @strSQL --Output the text being ran, can comment out
EXEC xp_cmdshell @strSQL

There we have it. A NetApp snap backup script which will continue to work even when you go crazy and decide to add/drop as many databases as you can.

Hopefully there are other out there that can benefit from this script 🙂

Feel free to leave constructive criticism or even a positive comment but please don’t comment on things like “why have you used xp_cmdshell?” or “why did you do it so that you have to run a separate SET statement to remove trailing comma’s”, everyone has their way and this is invariably is mine 

UPDATE (30th Nov 2011):
The latest versions of Snapmanager for SQL have addressed this issue and now if you select all DBs to backup it does not output all DB names into the job script as per the above. You can still use the script above to dynamically backup a selected set of databases by amending the select from sys.databases query so its not completed redundant!!!

Why I don’t like NULLs and Nullable Columns — March 25, 2011

Why I don’t like NULLs and Nullable Columns

As I imagine (or hope), the majority of us adhere to some form of standards in the environment that they work in. Whether that be whether you have to wear a Shirt and Tie, be in for 9am or those of us in a Database or Development environment, Coding Standards.

Everyone who has worked with me knows that one of my standards is NO NULLABLE columns in any new development unless there is an extremely good, valid reason and to date, I’ve only ever had 1 developer convince me that they should make their column nullable – and that was due to the current system design meaning we had to have it without a complete re-write of that part of the system. I don’t really want this blog to turn into a big debate as to whether I’m right or wrong on this, its purely my view based on the many years I’ve been working with SQL Server and development teams.

There are many blogs out there where DBAs share the same or similar views such as Thomas LaRock (Blog|Twitter) in which he talks about the harm they cause by the result of a person not knowing they are there. What I’m going to focus on in this blog is more where people know the NULL values are there and tailor their T-SQL around it, specifically when the value is NULL and they don’t want to show NULL on screen so they replace it with another value.

I’ve knocked up a quick script to populate a table EMPLOYEE with dummy data for the purposes of this blog and yes, I’ve purposely left MODIFIEDBY_ID and MODIFIEDBY_DATE as NULLable and no, i’m not saying this is the table design anyone should be using in their systems, especially later on.

CREATE TABLE EMPLOYEE
   (
      EMP_ID INTEGER IDENTITY(1000,1)
                     PRIMARY KEY
                     NOT NULL
     ,DATE_OF_BIRTH DATETIME NOT NULL
     ,SALARY DECIMAL(16,2) NOT NULL
     ,EMP_NAME VARCHAR(50) NOT NULL
     ,INSERTED_ID INT NOT NULL
     ,INSERTED_DATE DATETIME NOT NULL
     ,MODIFIEDBY_ID INT NULL
     ,MODIFIEDBY_DATE DATETIME NULL
   )
GO

SET NOCOUNT ON ;

DECLARE @Counter BIGINT ;
DECLARE @NumberOfRows BIGINT
SELECT  @counter = 1
       ,@NumberOfRows = 70000
WHILE ( @Counter < @NumberOfRows )
      BEGIN
            INSERT  INTO EMPLOYEE
                    (
                      DATE_OF_BIRTH
                    ,SALARY
                    ,EMP_NAME
                    ,INSERTED_ID
                    ,INSERTED_DATE
                    )
                    SELECT  CONVERT(VARCHAR(10) , getdate() - ( ( 18 * 365 ) + RAND() * ( 47 * 365 ) ) , 103)
                           ,CONVERT(DECIMAL(16,2) , ( 50000 + RAND() * 90000 ))
                           ,'Dummy Name' + CONVERT(VARCHAR(100) , @Counter)
                           ,CONVERT(INT , rand() * 10000)
                           ,CONVERT(VARCHAR(10) , getdate() - ( ( 18 * 365 ) + RAND() * ( 47 * 365 ) ) , 103)

            SET @Counter = @Counter + 1
      END ; 
     
SELECT  @counter = 1
       ,@NumberOfRows = 25000
WHILE ( @Counter < @NumberOfRows )
      BEGIN
            INSERT  INTO EMPLOYEE
                    (
                      DATE_OF_BIRTH
                    ,SALARY
                    ,EMP_NAME
                    ,INSERTED_ID
                    ,INSERTED_DATE
                    ,MODIFIEDBY_ID
                    ,MODIFIEDBY_DATE
                    )
                    SELECT  CONVERT(VARCHAR(10) , getdate() - ( ( 18 * 365 ) + RAND() * ( 47 * 365 ) ) , 103)
                           ,CONVERT(DECIMAL(16,2) , ( 50000 + RAND() * 90000 ))
                           ,'Dummy Name' + CONVERT(VARCHAR(100) , @Counter)
                           ,CONVERT(INT , rand() * 10000)
                           ,CONVERT(VARCHAR(10) , getdate() - ( ( 18 * 365 ) + RAND() * ( 47 * 365 ) ) , 103)
                           ,CONVERT(INT , rand() * 10000)
                           ,CONVERT(VARCHAR(10) , getdate() - ( ( 18 * 365 ) + RAND() * ( 47 * 365 ) ) , 103)

            SET @Counter = @Counter + 1
      END ;    

I also create a couple of non-clustered indexes that should be utilised by the sample queries below:

CREATE INDEX idx1 ON employee ( MODIFIEDBY_DATE , inserted_date ) INCLUDE ( emp_id ,salary )
CREATE INDEX idx2 ON employee ( MODIFIEDBY_DATE ) INCLUDE ( emp_id ,salary )

So, the table is there and the data populated, now for one of my biggest bugbears:

Query 1:

SELECT  emp_id
       ,salary
       ,isnull(MODIFIEDBY_DATE , inserted_date)
FROM    EMPLOYEE
WHERE   isnull(MODIFIEDBY_DATE , inserted_date) > dateadd(yy , -20 , getdate())

Arrrrgggghhhh!!!! Don’t get me wrong, i do understand why tables and columns like this do exist but for a DBA its a nightmare to tune as any index you may put on these columns will ultimately end in a Scan as opposed to a Seek.

Here is the execution plan:

As a comparison here is the query and execution plan for the query without the ISNULL function:

Query 2:

SELECT  emp_id
       ,salary
       ,MODIFIEDBY_DATE
FROM    EMPLOYEE
WHERE   MODIFIEDBY_DATE > dateadd(yy , -20 , getdate())

Execution Plan:

And a comparison of the two:

SELECT  emp_id
       ,salary
       ,isnull(MODIFIEDBY_DATE , inserted_date)
FROM    EMPLOYEE
WHERE   isnull(MODIFIEDBY_DATE , inserted_date) > dateadd(yy , -20 , getdate())

SELECT  emp_id
       ,salary
       ,MODIFIEDBY_DATE
FROM    EMPLOYEE
WHERE   MODIFIEDBY_DATE > dateadd(yy , -20 , getdate())

Thats 95% against 5%!! What a difference!

Now, for bugbear #2 i’ll need to add a new column with new values, again, i’m not saying you should or would ever do this but for the purposes of the blog:

ALTER TABLE EMPLOYEE
ADD   PreviousSalary DECIMAL(16,2)

So i’ve now added a new column with all NULL values, however, i don’t want my users seeing the word NULL on the front end. Easy, i’ll replace it with a zero:

SELECT  emp_id
       ,salary
       ,isnull(MODIFIEDBY_DATE , inserted_date) AS LastModifiedDate
       ,isnull(PreviousSalary,0) AS PreviousSalary
FROM    EMPLOYEE
WHERE   isnull(MODIFIEDBY_DATE , inserted_date) > dateadd(yy , -20 , getdate())

From this you can now see that the SQL Optimiser will now ignore the index created and scan the clustered index:

Obviously i could amend idx1 to factor in this column or create a new index:

CREATE INDEX idx3 ON employee ( MODIFIEDBY_DATE , inserted_date ) INCLUDE ( emp_id ,salary, PreviousSalary )

And sure enough it will choose that index but again, its a scan!

Ok, so you may well be thinking that this doesn’t happen often or wouldn’t cause too much of an issue on your system which may very well be correct. However, what about a highly transactional financial system with 100’s of millions of rows? Yes it does happen! What makes matters even worse is if you decided you wanted to search for (in this example) PreviousSalary > 10000, you’d then have to handle the NULL values and convert them to another value (typically 0) which begs the question, why is it not set to an arbitrary default value in the first place?

I’ve updated a random selection of PreviousSalary records to now have non-NULL values and added a new index:

CREATE INDEX idx4 ON employee ( PreviousSalary ) INCLUDE ( emp_id ,salary )

Running the query to handle the NULLs will still produce and Index Scan:

SELECT  emp_id
       ,salary
       ,isnull(PreviousSalary,0)
FROM    EMPLOYEE
WHERE   isnull(PreviousSalary,0) > 10000   

If I now update all the NULL columns to 0 (I could have used an arbitrary figure such as -1, -100 or even -99999 to indicate a NULL value) and amend the query above, we now get the execution plan:

SELECT  emp_id
       ,salary
       ,isnull(PreviousSalary,0)
FROM    EMPLOYEE
WHERE   isnull(PreviousSalary,0) > 10000   

SELECT  emp_id
       ,salary
       ,PreviousSalary
FROM    EMPLOYEE
WHERE   PreviousSalary > 10000

I think by now everyone will be getting the picture. So how do you rectify this? First and foremost its education. Speak with your developers, do “Lunch ‘n’ Learn” sessions, anything you can to express how much of an issue this can potentially be and make the time to sit with them and (re)design that part of the system so that you don’t have to handle NULLs either at the query level or even the front end. If they understand that if they use the ISNULL function in this way then their query will not be SARGable and ultimately hurt performance, I’m confident that you won’t have any issues converting them to this method.

I know that the majority of people who may read this may not take anything away from it but even if I get a handful of people who can take this and speak with their development teams on this very simple concept of database design then you could save yourself major headaches in the future!