Friday, March 23, 2012

Infinite Loop in cursor

Hi

I have an infinite loop in a trigger I and I cant reslove it.

In my system the user updates a stock table from the GUI and on the update I need to check values to see if I need to add records to a StockHistory table. For Example: If the user changes the grade of Product X from A to B then I need to add a new line in StockHistory for product X grade A that decrements the total number of products in the warehouse. Similary I need to increase the quantity of stock for Product X grade B.
I had the trigger working for single updates but now when stock is added to the database (from another db) it has status of 'New'. This isn't actually 'in stock' until the user sets the status to 'Goods In'. This process will then update the status for all records in the category. This caused my trigger to fail as the 'inserted' table now contains many records.
Now the problem I have is the trigger is in an infinite loop. It always shows the id of the first record it finds and the @.Quantity values increases as expected. I've taken all my procesing code out of the trigger and adding some debugging stuff but it still doesnt work:
CREATE TRIGGER [StockReturns_on_change] ON [dbo].[StockReturns]
FOR UPDATE
AS
DECLARE INDIVIDUAL Cursor Cursor for all the rows being updated
FOR
SELECT Id FROM inserted
OPEN INDIVIDUAL
FETCH NEXT FROM INDIVIDUAL INTO @.Id
select @.Quantity = 1
print @.@.FETCH_STATUS
print @.Id
print @.Quantity
WHILE @.@.FETCH_STATUS = 0
begin
select @.Quantity = @.Quantity + 1
print @.@.FETCH_STATUS
print @.Id
print @.Quantity
-- Get the next row from the inserted table
FETCH NEXT FROM INDIVIDUAL INTO @.Id
End -- While loop on the cursor
-- no close off the cursors
CLOSE INDIVIDUAL
DEALLOCATE INDIVIDUAL


Can you help me please?
Kind Regards

i think this is not an infinite query but a "long running" query. its going to finish

its just taking a lot of time

i suggest you get rid of the cursor replace it with a faster and better update code that process multiple records at the same time

use aggregate function(sum, count,min, max.. etc) in place of your counters (@.x=@.x+1)

cursors are very slow way of doing things

regards,

|||

Thanks for the reply.

Nop its an infinite loop. The print statements show that id is always the same and the quanityt counter is being incremented showing that its travelling round the cursor. Or are you saying its looping around but still waiting for the first transaction to finish? I went out yesterday for 20 minutes and it was still showing the 1st record.

If I knew what code to write that would work then id do it. Unfortunatley this is the only way I can think of doing it.

Each record needs to be treated seperately as i have to inpect the values of 2 variables (both in inserted and deleted) to see if they have changed for each record. I cannot do a bulk insert.

|||

here are your watch point

1. cursors are realy slow

2. maybe the triggers are in recurssion. meaning this trigger is fired by an update event of table1. in case the "update triggers" updates the same table (table1) again its going to call the same update trigger again and the cycle go an and on until 32 level deep per record.

if thats the case it will realy take a while to finish

solution:

you can write queries or correlated subqueries that joins your inserted and deleted table with the base tables to do the comparison.

such as

update basetable1 set fieldname = select count(xx) from

from inserted where inserted.id=basetable.id

in this way you do a one way trip to the server so even if it will be in recursion it is still fast

|||

this trigger is on the stock table and it doesnt insert or update anything. Check the code i posted. It merely attempts to get the next value from the "inserted" table. I'm running that exact code and it does loop forever.

Heres dome debug ive collected whilst running the sql "Update stockreturns set itemstatus = 1 where id = 26301": (so its only updating 1 record)

0 fetch status
26301 id updating
1 loop counter
0
26301
2
0
26301
3
0
26301
4
0
26301
5
0
26301
6
0
26301
7
0
26301
8
0
26301
9
etc

Regards

|||

i've modified your trigger and apply it to northind. employees

here's the code


use northwind

CREATE TRIGGER [StockReturns_on_change] ON [dbo].[employees]
FOR UPDATE
AS
declare @.id int
declare @.quantity int
DECLARE INDIVIDUAL Cursor Cursor for all the rows being updated

FOR
SELECT employeeId FROM inserted

OPEN INDIVIDUAL

FETCH NEXT FROM INDIVIDUAL INTO @.Id

select @.Quantity = 1

print @.@.FETCH_STATUS
print @.Id
print @.Quantity

WHILE @.@.FETCH_STATUS = 0
begin

select @.Quantity = @.Quantity + 1

print @.@.FETCH_STATUS
print @.Id
print @.Quantity

-- Get the next row from the inserted table
FETCH NEXT FROM INDIVIDUAL INTO @.Id

End -- While loop on the cursor

-- no close off the cursors
CLOSE INDIVIDUAL
DEALLOCATE INDIVIDUAL

go

update employees set lastname='joey' where employeeid=1

and heres the result

0
1
1
0
1
2

|||

now i've tried this

begin transaction
update employees set lastname='joey' where employeeid<6

and here's the result

0
1
1
0
1
2
0
2
3
0
3
4
0
4
5
0
5
6

(5 row(s) affected)

meaning this is not a infinite loop but a slow running query. how many records are you updating. by the way whats your requirements?


|||

Hi Joey

Thanks for your efforts in trying to get me to understand!!

My requirements, ok here goes.

The user prints a stock manifest when stock comes into the warehouse which copies all the records to do with that manifest (store and date) from another database to my stock database. When these records are copied they are copied with a statusid of 0 (new stock). The manifest can have anywhere between 1 and 100 products on it.

The warehouse will then classify each product into grades A,B,C or D. The products by default are grade C in the database. After the products for this manifest have been classified a "goods in confirmation" report is produced which updates all the products on that manifest from statusid = 0 to statusid = 1 (goods in). Meaning that only now will they appear on stock reports and can be picked for despatched.

Now, we have a table called stockHistory which holds movements for every product (piece of equipment). Only when the products are cliassified as statusid = 1 do they offically enter the warehouse and so the historic table needs to be updated to include these products. Historic data is never changed or deleted. Only inserts are allowed. The historic table also holds the total number of each product in stock so when we are adding Product X Grade C to the warehouse we get the max(id) for Product X Grade C and increment the total quantity value by 1 as we are added a product to the warehouse of that type.

But, the stock can also go to Despatched which means we decrement a value. Despatched items have status of 5.

Also the grade of the stock can change which will mean we need to decrement from the old grade and increment from the new grade.

My trigger before i tried to get it working looked like:

SELECT @.newItemState = (SELECT itemStatus FROM Inserted)
SELECT @.oldItemState = (SELECT itemStatus FROM deleted)
select @.OldGrade = (select Grade from deleted) -- fetch the product id
select @.Grade = (select Grade from inserted) -- fetch the product id

DECLARE @.State tinyint

-- If its a product going to stock then insert
if (@.oldItemState = 0 and @.newItemState = 1)
begin
select @.State = 1
end

-- If the grade has changed we need to remove from old grade and add to new grade
else if (@.newItemState = 1 and @.Grade <> @.OldGrade)
begin
select @.State = 2
end

-- If the item has been dispatched remove from new grade
else if (@.newItemState = 5)
begin
select @.State = 3
end

so then i increment the current value on state 1 and 2 and decrement the value of the old product on state 2 and 3.

So if its state 1 or 3 then you use the values contained in "inserted" otherwise if state = 2 then i need to use the record of the old product grade "deleted" to decrement the quantity and use the new product grade to increment the quantity.

Damn that was hard work explaining, I hope that makes your understanding of my problem easier!!

At first I was under the impression that the trigger would fire for each SINGLE update but when I found out it didnt I thought looping round the inserted table was the thing to do.

So why does my loop go on forever and yours stops. Both bits of code are reading from the "inserted" table and simply printing the "id". We arent updating any other tables to cause a problem. Is there a setting in SQL 2000 which I have switched off?

Regards

|||

You don't need a cursor loop to perform this logic. You can just do it via DML statements alone.

-- If its a product going to stock then insert

insert into ....

select ....

from inserted as i

join deleted as d

on d.key_col = i.key_col

where d.itemStatus = 0 and i.itemStatus = 1

-- If the grade has changed we need to remove from old grade and add to new grade
/* I am not sure if remove means update existing row or delete from table. */

Anyway, I hope you get the idea. Using cursor loop for this is probably overkill and has performance implications. It is much simpler to write series of DML statements by inspecting the rows of inserted/deleted tables as necessary. If you need more help then please post some sample schema, data and expected results for one particular case. You can then do the same for the rest.

|||

OK cheers for that but Ive got one thing I have forgotten to say.

The stockHistory table contains 2 columns that come from a different table when the product is being despatched.

When a product is despatched the advicenote and processorid both need to be from the picklist table.

The stock table holds the fk to the picklist table called picklistid. From the picklist table I need the processorid and the despatchnote which get inserted into the processorid and advicenote columns respectively of the stockHistory table.

So I don't think I can simply do a bulk insert using data straight from the inserted table. Or can I?

Thanks

|||

UNBELIEVABLE!!

Joey, I just tried my code against the nortwind.employees table and it looped continuously. I tried your code and it worked.

I then have put your code against my stock table and run the upodate and it worked.

I've compared yours against mine and they are (as far as I can see) identical.

Im going to give it a go now and see what happens!!

Cheers

|||

IVE GOT IT.

THE LINE

-- Get the next row from the inserted table

causes it to forever loop. Without this it works fine!

|||

All sorted now and the trigger works fine.

Cheers for testing out my code Joey.

No comments:

Post a Comment