Stored Procedure enhancement for updating columns in a subset

Question

The purpose of this store procedure is to reach into a large database (test), query for a subset, and update a column value, but with a big constraint - I can only update one row at a time in order to avoid deadlocks because the database is large and used by many resources.

The code attempts to change the value of test.[stat] to 'R' if a subset is found with the corresponding stat values.

ALTER PROCEDURE [dbo].testProcedure (@name NVARCHAR(10), 
 @num INT,
 @msg NVARCHAR(200) OUTPUT )
AS
BEGIN
 DECLARE @val1 NVARCHAR(10) 
 DECLARE @val2 INT
 SELECT t.val1, t.val2, t.stat, 'N' rel
 INTO #temp
 FROM dbo.test t
 WHERE t.[name] = @name
 AND t.[stat] IN ('A','B') 
 AND t.[num] = @num
 IF @@ROWCOUNT = 0
 BEGIN
 SET @msg = 'Nothing returned'
 PRINT @msg
 RETURN -1
 END
 SELECT @val1 = val1 FROM #temp where rel = 'N'
 SET @msg = ' val1: ' + @val1 + ' '
 WHILE EXISTS (SELECT 1 from #temp WHERE rel = 'N')
 BEGIN
 SELECT @val2 = val2 FROM #temp where rel = 'N'
 UPDATE t
 SET [stat] = 'R'
 FROM dbo.test t
 WHERE t.val1 = @val1 AND t.[stat] IN ('A', 'B') AND t.val2 = @val2
 UPDATE #temp
 SET rel = 'Y'
 WHERE val2 = @val2
 SET @msg = @msg + ' - ' + ISNULL(CONVERT(NVARCHAR,@val2),' ') 
 END
 IF OBJECT_ID('tempdb..#tempd') IS NOT NULL DROP TABLE #temp
 PRINT @msg
END

Any help on how to improve this code will be greatly appreciated.

• Phrancis: Is there any specific reason you are constrained to updating only one row at a time? Generally that's the least efficient method of performing writes on table rows. The only good reason that I can think of is to minimize locks (usually at the expense of execution time).
• mugabits: Exactly that. The database is large and needed by plenty of resources, so it is to avoid deadlocks.
• Phrancis: Makes sense. I would suggest to add this clarification into your question.
• Phrancis: Is this your real query? It looks very much like example code to me, and SET [stat] = 'R', FROM dbo.test t (line 35-36) would raise a syntax error. Please clarify, as example code is off-topic on this site. If this is not the real, working code, please update the question to contain the real code.
• mugabits: It is my real code, I just changed all variable names.

Answer 1

Anonymized code

Object/variable names should tell us something about what they actually mean. For instance, these variables:

 DECLARE @val1 NVARCHAR(10) 
 DECLARE @val2 INT

What does val1 and val2 actually stand for? In fact everything in the query looks like example code, so it leaves us to guess what the code is actually designed for.

---

SELECT INTO #table

You would most likely get performance improvements with regards to that temp table if you created it manually instead of using SELECT ... INTO. That would at least let you create some indexes on it. I've gained huge performance improvements with that technique in the past so I strongly recommend explicitly creating your temp tables along with indexes for performance-critical queries.

---

Cursor

You would likely get better performance in row-by-row operations if you used a cursor, instead of doing an update both to the source table and the temp table in a while loop. See cursors on MSDN for reference.

---

Print @msg

I'm not really sure why you would print the @msg variable. According to your question, the purpose of the code is:

The code attempts to change the value of test.[stat] to 'R' if a subset is found with the corresponding stat values.

So printing that message to the console seems like a waste of resources. I notice you are returning @msg as output anyways, so if the calling code needs that, it's easy to get it without printing it to the console.