236

I have a stored procedure that alters user data in a certain way. I pass it user_id and it does it's thing. I want to run a query on a table and then for each user_id I find run the stored procedure once on that user_id

How would I write query for this?

HaveNoDisplayName
  • 8,291
  • 106
  • 37
  • 47
MetaGuru
  • 42,847
  • 67
  • 188
  • 294
  • 5
    You need to specify what RDBMS - the answer will be different for SQL Server, Oracle, MySql, etc. – Gary.Ray May 20 '09 at 05:33
  • 5
    Chances are that you don't need a stored procedure at all. Can you outline "what" the stored procedure does, exactly? Maybe the whole process can be expressed as a single update statement. The "do once for each record" pattern should generally be avoided, if possible. – Tomalak May 20 '09 at 05:38
  • Which database are you using? – Rashmi Pandit May 20 '09 at 05:38
  • 1
    You should read this article... item 2 says DON'T use cursors http://www.codeproject.com/KB/database/sqldodont.aspx...mind I'm also against premature optimization. – Michael Prewecki May 20 '09 at 06:26
  • http://www.codeproject.com/KB/database/sqldodont.aspx try that again – Michael Prewecki May 20 '09 at 06:27
  • 8
    @MichaelPrewecki: If you read further in that poorly written article, you'll see that item 10 is "DON'T use server side cursors Unless you know what your are doing." I think this is a case of "I know what I'm doing". – Gabe Jan 14 '13 at 06:05
  • It's best if you can avoid this. Most databases are optimized for set based actions, and looping using a cursor will be relatively slow. That said, here is a good example for how to do this using [SQL Server](http://weblogs.asp.net/jgalloway/archive/2006/04/12/442618.aspx) – Gary.Ray May 20 '09 at 05:41
  • @MichaelPrewecki "*They should be your preferred way of killing the performance of an entire system.*" Hilarious. I mean, this matches my use case exactly. – ruffin Apr 25 '16 at 18:56

7 Answers7

277

use a cursor

ADDENDUM: [MS SQL cursor example]

declare @field1 int
declare @field2 int
declare cur CURSOR LOCAL for
    select field1, field2 from sometable where someotherfield is null

open cur

fetch next from cur into @field1, @field2

while @@FETCH_STATUS = 0 BEGIN

    --execute your sproc on each row
    exec uspYourSproc @field1, @field2

    fetch next from cur into @field1, @field2
END

close cur
deallocate cur

in MS SQL, here's an example article

note that cursors are slower than set-based operations, but faster than manual while-loops; more details in this SO question

ADDENDUM 2: if you will be processing more than just a few records, pull them into a temp table first and run the cursor over the temp table; this will prevent SQL from escalating into table-locks and speed up operation

ADDENDUM 3: and of course, if you can inline whatever your stored procedure is doing to each user ID and run the whole thing as a single SQL update statement, that would be optimal

Community
  • 1
  • 1
Steven A. Lowe
  • 60,273
  • 18
  • 132
  • 202
  • 24
    You missed off 'open cur' after the declaration - this was giving me 'the cursor is not open' errors. I don't have the rep to do an edit. – Fiona - myaccessible.website Mar 24 '10 at 14:16
  • 5
    You can thank people by up-voting their comment. Who knows, maybe that way they'll have the rep to make the edit, next time! :-) – Robino Jun 25 '14 at 07:57
  • Make sure you check your indexes on the JOINS and WHERE clauses in the fields used in your stored procedure. I dramatically sped up calling my SP in a loop after adding appropriate indexes. – Matthew Feb 16 '18 at 06:32
  • 2
    Thanks for the reminder of using temp table to avoid potential locking problems caused by long execution. – Tony Aug 17 '18 at 18:09
  • Sometimes the stored procedure is too large or complicated to inline without risking introducing bugs. Where performance isn't the ultimate priority, executing the SP in a cursor loop is often the most practical choice. – Suncat2000 Jan 23 '19 at 12:54
58

try to change your method if you need to loop!

within the parent stored procedure, create a #temp table that contains the data that you need to process. Call the child stored procedure, the #temp table will be visible and you can process it, hopefully working with the entire set of data and without a cursor or loop.

this really depends on what this child stored procedure is doing. If you are UPDATE-ing, you can "update from" joining in the #temp table and do all the work in one statement without a loop. The same can be done for INSERT and DELETEs. If you need to do multiple updates with IFs you can convert those to multiple UPDATE FROM with the #temp table and use CASE statements or WHERE conditions.

When working in a database try to lose the mindset of looping, it is a real performance drain, will cause locking/blocking and slow down the processing. If you loop everywhere, your system will not scale very well, and will be very hard to speed up when users start complaining about slow refreshes.

Post the content of this procedure you want call in a loop, and I'll bet 9 out of 10 times, you could write it to work on a set of rows.

KM.
  • 101,727
  • 34
  • 178
  • 212
  • 3
    +1 for a very good workaround, assuming that you control the child sproc – Steven A. Lowe May 24 '09 at 03:51
  • a bit of thinking, this sollution is by far superior! – encc Nov 07 '13 at 09:22
  • 7
    Set based operations are always preferable. However, keep in mind that modifying an SP is not always an option - think vendor provided solutions. Some users may not even have visibility, leaving only the cursor or loop options. In my shop, our devs can see everything but there are a lot of hurdles to clear if a solution is built outside of the vendor's application due to triggers, nested procs, number of records being manipulated etc. Many times their best option, due to complexity of the application, is to simply cursor through the records. – Steve Mangiameli Feb 08 '17 at 16:37
  • One of the most usefully presented answers I've seen on SO - thanks – Andy Brown Mar 03 '23 at 15:05
15

You can do it with a dynamic query.

declare @cadena varchar(max) = ''
select @cadena = @cadena + 'exec spAPI ' + ltrim(id) + ';'
from sysobjects;
exec(@cadena);
JJJ
  • 32,902
  • 20
  • 89
  • 102
Dave Rincon
  • 308
  • 3
  • 10
12

Something like this substitutions will be needed for your tables and field names.

Declare @TableUsers Table (User_ID, MyRowCount Int Identity(1,1)
Declare @i Int, @MaxI Int, @UserID nVarchar(50)

Insert into @TableUser
Select User_ID
From Users 
Where (My Criteria)
Select @MaxI = @@RowCount, @i = 1

While @i <= @MaxI
Begin
Select @UserID = UserID from @TableUsers Where MyRowCount = @i
Exec prMyStoredProc @UserID
Select

 @i = @i + 1, @UserID = null
End
u07ch
  • 13,324
  • 5
  • 42
  • 48
8

Use a table variable or a temporary table.

As has been mentioned before, a cursor is a last resort. Mostly because it uses lots of resources, issues locks and might be a sign you're just not understanding how to use SQL properly.

Side note: I once came across a solution that used cursors to update rows in a table. After some scrutiny, it turned out the whole thing could be replaced with a single UPDATE command. However, in this case, where a stored procedure should be executed, a single SQL-command won't work.

Create a table variable like this (if you're working with lots of data or are short on memory, use a temporary table instead):

DECLARE @menus AS TABLE (
    id INT IDENTITY(1,1),
    parent NVARCHAR(128),
    child NVARCHAR(128));

The id is important.

Replace parent and child with some good data, e.g. relevant identifiers or the whole set of data to be operated on.

Insert data in the table, e.g.:

INSERT INTO @menus (parent, child) 
  VALUES ('Some name',  'Child name');
...
INSERT INTO @menus (parent,child) 
  VALUES ('Some other name', 'Some other child name');

Declare some variables:

DECLARE @id INT = 1;
DECLARE @parentName NVARCHAR(128);
DECLARE @childName NVARCHAR(128);

And finally, create a while loop over the data in the table:

WHILE @id IS NOT NULL
BEGIN
    SELECT @parentName = parent,
           @childName = child 
        FROM @menus WHERE id = @id;

    EXEC myProcedure @parent=@parentName, @child=@childName;

    SELECT @id = MIN(id) FROM @menus WHERE id > @id;
END

The first select fetches data from the temporary table. The second select updates the @id. MIN returns null if no rows were selected.

An alternative approach is to loop while the table has rows, SELECT TOP 1 and remove the selected row from the temp table:

WHILE EXISTS(SELECT 1 FROM @menuIDs) 
BEGIN
    SELECT TOP 1 @menuID = menuID FROM @menuIDs;

    EXEC myProcedure @menuID=@menuID;

    DELETE FROM @menuIDs WHERE menuID = @menuID;
END;
Erk
  • 1,159
  • 15
  • 9
6

Can this not be done with a user-defined function to replicate whatever your stored procedure is doing?

SELECT udfMyFunction(user_id), someOtherField, etc FROM MyTable WHERE WhateverCondition

where udfMyFunction is a function you make that takes in the user ID and does whatever you need to do with it.

See http://www.sqlteam.com/article/user-defined-functions for a bit more background

I agree that cursors really ought to be avoided where possible. And it usually is possible!

(of course, my answer presupposes that you're only interested in getting the output from the SP and that you're not changing the actual data. I find "alters user data in a certain way" a little ambiguous from the original question, so thought I'd offer this as a possible solution. Utterly depends on what you're doing!)

randomsequence
  • 1,338
  • 1
  • 11
  • 14
  • 2
    OP: "stored procedure that alters user data in a certain way" [MSDN](http://technet.microsoft.com/en-us/library/ms191320.aspx):User-defined functions cannot be used to perform actions that modify the database state. However SQLSVR 2014 doesn't seem to have a problem with it – johnny 5 Dec 06 '14 at 01:03
3

I like the dynamic query way of Dave Rincon as it does not use cursors and is small and easy. Thank you Dave for sharing.

But for my needs on Azure SQL and with a "distinct" in the query, i had to modify the code like this:

Declare @SQL nvarchar(max);
-- Set SQL Variable
-- Prepare exec command for each distinctive tenantid found in Machines 
SELECT @SQL = (Select distinct 'exec dbo.sp_S2_Laser_to_cache ' + 
              convert(varchar(8),tenantid) + ';' 
              from Dim_Machine
              where iscurrent = 1
              FOR XML PATH(''))

--for debugging print the sql 
print @SQL;

--execute the generated sql script
exec sp_executesql @SQL;

I hope this helps someone...

Tom
  • 113
  • 8