Do I need a cursor? ...SQL Server

bigbadcon

Hi guys,

Just wondering if there is a way to avoid using a cursor for what I want to do..
I have it working with a cursor but I know its good to avoid them where possible..


I want to list the products in the product table and also show the SUM of the damaged Products and then the SUM in good condition.

Sample Row returned

ProductId 123456
TotalOfThisProductInStock 100
TotalProductsIngoodCondition 75
TotalDamagedProducts 25


I have a cursor that is getting each product in the product table and then doing 2 seperate queries inside the cursor (one for damaged products and one for non damaged products) and inserting the values into a table variable for each product.

Im then reading from the table variable at the end of the Stored Procedure..

Is there a better way of doing this?

Thanks for your help...

Evil Phil

Can you post your sproc code? It will help to illustrate what exactly you are trying to achieve.

bigbadcon

My code is a bit longer and I have changed it around a bit but this should give you the jist of the cursor

DECLARE product_cursor CURSOR 
    FOR 
	
		SELECT productId from ProductTable

OPEN product_cursor
FETCH NEXT FROM product_cursor
INTO @currentProductId

WHILE @@FETCH_STATUS = 0
BEGIN


			SET @TotalDamagedProducts = ( SELECT  SUM(productState) from ProductTable
where productId = @currentProductId
AND productState = 'Damaged'
GROUP BY productID)


			SET @TotalProductsGoodCondition = ( SELECT  SUM(productState) from ProductTable
where productId = @currentProductId
AND productState <> 'Damaged'
GROUP BY productID)

			SET @TotalProducts = ISNULL(@TotalDamagedProducts,0) + ISNULL(@TotalProductsGoodCondition ,0)
			

	INSERT into @OutputData
	Values(@currentProductId,@TotalProducts ,ISNULL(@TotalDamagedProducts
			,0),ISNULL(@TotalProductsGoodCondition ,0))



FETCH NEXT FROM product_cursor
INTO @currentProductId
END
   

CLOSE product_cursor
DEALLOCATE product_cursor

select * from @OutputData

So the output is

ProductId,TotalProducts,TotalDamaged,TotalGoodCondition
123456,100,75,25
1234567,200,150,50

etc


thanks

stephenlane80

if you did it in a couple of steps like this you should get best performance, cursors have horrible performance on big tables:

DECLARE @tmpDamaged TABLE
(
  sumDamaged int,
  productID int
)

DECLARE @tmpGood TABLE
(
  sumGood int,
  productID int
)

INSERT INTO @tmpDamaged (sumDamaged, productID)
SELECT  SUM(productState), productID into @tmpDamaged from productTable
WHERE productState = 'Damaged'
GROUP BY productID;

INSERT INTO @tmpGood (sumGood, productID)
SELECT  SUM(productState), productID into @tmpGood from ProductTable
WHERE productState <> 'Damaged'
GROUP BY productID;

SELECT td.productID, td.sumDamaged, tg.sumGood, ISNULL(td.sumDamaged,0) + ISNULL(tg.sumGood ,0)
FROM
@tmpDamaged td, @tmpGood tg
WHERE td.productID = tg.productID

i havent run this code so you may have to tinker !

showry

what about a case statement?

select id,
sum(case when state = 'damaged' then 1 else 0 end) as 'damaged',
sum(case when state <> 'damaged' then 1 else 0 end) as 'good',
count(*) as 'total'
from products
group by id;

stephenlane80

showry wrote: »

what about a case statement?

i like the look of this, it would be interested to see a performance test between the cursor and the other 2 suggestions

kayos

stephenlane80 wrote: »

i like the look of this, it would be interested to see a performance test between the cursor and the other 2 suggestions

The case statement will be the better of the 2 suggestions. The use of table variables and the associated inserts along and then the join between the two tables are all extra commands that really dont need to be there.

The cursor could well be the faster option if the dataset is small and will for ever stay that way. But on the other hand I would avoid cursors as much as possible.

bigbadcon

Hi guys,

thanks for the replies.The case statement looks good but I simplified my code for the example and it actually has joins to a few tables with where clauses etc so im not sure If i can add this in the case statement...

Will keep it in mind if im doing the same thing on just one table..

The 2 table option is the one I used in the end and the speed of the SP has gone from over a minute to less than a second


Saying that I copped a huge bottleneck in the code that was causing the extra processing so its not really comparing like with like..

Either way im happy and I appreciate all the suggestions..

Out of interest is there any situation where you MUST use a cursor?

kayos

bigbadcon wrote: »

The case statement looks good but I simplified my code for the example and it actually has joins to a few tables with where clauses etc so im not sure If i can add this in the case statement...

The 2 table option is the one I used in the end and the speed of the SP has gone from over a minute to less than a second

The case statement could be used in the situation you describe and will always be faster than having to perform extra statements such as the inserts used in the table variable way.

bigbadcon wrote: »

Out of interest is there any situation where you MUST use a cursor?

Nope you could always use a while loop :P.

But to be serious there are times you will have no other option than to go the cursor route. But for the majority of the times they can and should be avoided. When you see a system with large numbers of cursors the odds are it was programmers with little exp of SQL and use their normal programming approach and loop the data.

stephenlane80

bigbadcon wrote: »

Hi guys,

Out of interest is there any situation where you MUST use a cursor?

If you need to work out complicated metrics this could br the case, i remember having no other alternative than to use one on a complicated percentile calculation problem,

But this type of logic is best kept in the application layer rather than the database, it a good rule of thumb to just perform relation operations in the database, but sometimes it isnt practical