Blog Post

Following good practice saves wasted time down the line.

,

G’day,

Sometimes I come across what I call unusual coding practices. Developers usually have pressure on them to deliver and this often results in simply delivering code that works – with very little attention to best practice , performance or security.

Sometimes this lack of attention to best practice (or even good practice), leaves substantial scope for the introduction of subtle bugs into code.

Take the case of two stored procedures where one stored procedure (sp1) produces a temporary table and loads it with data, the calls a second stored procedure (sp2) which selects data from this table.

Yes, I can guess what you are thinking “why not just do all these operations in a single stored procedure”

Well here some code to demonstrate*

USE [tempdb];
GO
IF EXISTS (SELECT 1 FROM [sys].[databases] [D] WHERE [D].[name] = 'SpTest')
BEGIN
DROP DATABASE [SpTest];
END
GO
CREATE DATABASE [spTest]
GO
USE [spTest];
GO
CREATE PROCEDURE [sp2]
AS
BEGIN
TRUNCATE TABLE #table1;
INSERT INTO [#table1]
SELECT [name], [database_id] FROM [sys].[databases] [D];
END;
GO
CREATE PROCEDURE [sp1]
AS
BEGIN
IF OBJECT_ID('Tempdb.dbo.#table1') IS NOT NULL
BEGIN
DROP TABLE #table1;
END
SELECT
[D].[name],
[D].[database_id]
INTO
#table1
FROM
[sys].[databases] [D];
EXEC [sp2];
SELECT * FROM [#table1];
END;
GO
EXEC [spTest].[dbo].[sp1]
GO

If you run the above code you’ll get exactly the output you would expect – the listing of a temp table (#table1)  that contains the name and database id of all databases in the sys.databases catalog.

All is well so far (leaving aside the somewhat convoluted design)

This works fine for some time until somebody decides to change the query in sp2 to pull out databases with an id greater that 4.

Now the definition of sp2 is changed to the following

ALTER PROCEDURE [sp2]
AS
BEGIN
TRUNCATE TABLE #table1;
SELECT [name] , [database_id] INTO [#table1] FROM [sys].[databases] [D];
END;
GO

so executing sp1

EXEC [spTest].[dbo].[sp1]
GO

now gives an empty result set – so what happened.

It took a while to initially figure out, but sp2 is now creating a brand new table in it’s own scope

with exactly the same name as the temp table created in sp1 – this is essentially hiding the intended temp table, which is going out scope when sp2 exits and the filter is appearing not to be doing anything.

Changing sp2 to simply do an insert into the temp table fixes the problem as no new table is being created.

ALTER PROCEDURE [sp2]
AS
BEGIN
TRUNCATE TABLE #table1;
INSERT INTO [#table1]
SELECT [name] , [database_id] FROM [sys].[databases] [D]
WHERE [database_id] > 4
END;
GO

However, the more robust solution is to simply combine the logic into a single sp, although sometimes this is easier said than done.

The moral of the story – keep your temp tables in the scope.

Have a good day.

Cheers

Martin.

*This is a purely fictitious scenario, any similarities to real life are coincidental!

Rate

You rated this post out of 5. Change rating

Share

Share

Rate

You rated this post out of 5. Change rating