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!