Sql-server – Cursor with Begin and End Transaction

cursorssql-server-2012

I've got the following script that keeps hanging on the 2nd iteration through the loop.

BEGIN
BEGIN TRY
        BEGIN TRANSACTION
        DECLARE @itemID int

        DECLARE LoopCursor CURSOR FOR
        SELECT DISTINCT i.ItemID
        FROM Inventory i
        INNER JOIN Items it ON i.ItemID = it.ItemID
        WHERE i.ItemID IN (226, 231, 232, 233, 234, 235, 245, 247 ,249 ,250 ,253 ,254 ,255 ,257 ,258 ,268 ,270 ,271 ,273 ,286 ,287 ,291 ,293 ,299 ,303 ,304,
                            305, 306, 307, 308, 310, 311, 312, 313, 314, 316, 322, 323, 324, 331, 332, 333, 334, 335, 338, 339, 340, 341, 342, 343, 345, 346 ,347)
        AND it.[Serializable] = 0
        ORDER BY i.ItemID

        OPEN LoopCursor

        FETCH NEXT FROM LoopCursor
        INTO @itemID

        WHILE @@FETCH_STATUS = 0

            --Drop Temp tables if they exist.
            IF OBJECT_ID('tempdb..#TEMP') IS NOT NULL
               DROP TABLE #TEMP

            IF OBJECT_ID('tempdb..#INV_TEMP') IS NOT NULL
               DROP TABLE #INV_TEMP

            BEGIN
                    DECLARE @inventoryID int = 0,
                            @quantity int = 0,
                            @statusID int = 10,
                            @maxInvID int       
                    SET @inventoryID = (SELECT MAX(InventoryID) AS InventoryID FROM Inventory WHERE Inventory.ItemID = @itemID)

                    --Store Assets in temp table.
                    SELECT MAX(AssetID) AS AssetID, RoomID, COUNT(*) AS Quantity
                    INTO #TEMP
                    FROM Assets
                    WHERE InventoryID IN (SELECT InventoryID
                                            FROM Inventory
                                            WHERE ItemID = @itemID)
                    GROUP BY RoomID

                    --Update Assets with new Quantity value.
                    UPDATE Assets
                    SET Quantity = t.Quantity
                    FROM Assets a
                    INNER JOIN #TEMP t ON a.RoomID = t.RoomID
                    INNER JOIN Inventory i ON a.InventoryID = i.InventoryID
                    WHERE i.ItemID = @itemID

                    --Delete all other Assets with the same ItemID and that's not in the temp table.
                    DELETE a
                    FROM Assets a
                    INNER JOIN Inventory i ON a.InventoryID = i.InventoryID
                    INNER JOIN #TEMP t ON a.RoomID = t.RoomID
                    WHERE i.ItemID = @itemID
                    AND a.AssetID NOT IN (SELECT AssetID FROM #TEMP)

                    --Update Assets to have all the same InventoryID.
                    UPDATE Assets
                    SET Assets.InventoryID = @inventoryID, DateUpdated = GETDATE()
                    FROM Assets a
                    INNER JOIN Inventory i ON a.InventoryID = i.InventoryID
                    INNER JOIN #TEMP t ON a.RoomID = t.RoomID
                    WHERE i.ItemID = @itemID
                    AND a.AssetID IN (SELECT AssetID FROM #TEMP)

                    --Clean up Inventory now.

                    --Delete all Inventory with specific Statuses.
                    DELETE
                    FROM Inventory
                    WHERE ItemID = @itemID
                    AND StatusID IN (3, 6, 12, 14)

                    --Store Inventory records in a temp table
                    SELECT MAX(InventoryID) AS InventoryID, ItemID, StatusID, COUNT(*) AS Quantity
                    INTO #INV_TEMP
                    FROM Inventory
                    WHERE ItemID = @itemID
                    GROUP BY ItemID, StatusID

                    SELECT @maxInvID = MAX(InventoryID)
                    FROM #INV_TEMP

                    SELECT @quantity = t.Quantity
                    FROM #INV_TEMP t
                    WHERE StatusID = @statusID

                    --If there are no more of these items in inventory, change the status.
                    IF(@quantity = 0)
                        SET @statusID = 17

                    --Update the Quantity and Status for the given InventoryID and ItemID.
                    UPDATE Inventory
                    SET Quantity = @quantity,
                        StatusID = @statusID,
                        DateUpdated = GETDATE()
                    WHERE Inventory.InventoryID = @maxInvID 
                    AND Inventory.ItemID = @itemID

                    --Delete all the other Inventory records with the same ItemID
                    DELETE
                    FROM Inventory
                    WHERE InventoryID <> @maxInvID
                    AND ItemID = @itemID

            FETCH NEXT FROM LoopCursor
            INTO @itemID
        END

    CLOSE LoopCursor
    DEALLOCATE LoopCursor
    COMMIT TRANSACTION
END TRY
BEGIN CATCH
    IF @@TRANCOUNT > 0
        ROLLBACK TRANSACTION;

        DECLARE @ErrorNumber INT = ERROR_NUMBER();
        DECLARE @ErrorLine INT = ERROR_LINE();
        DECLARE @ErrorMessage NVARCHAR(4000) = ERROR_MESSAGE();
        DECLARE @ErrorSeverity INT = ERROR_SEVERITY();
        DECLARE @ErrorState INT = ERROR_STATE();

        PRINT 'Actual error number: ' + CAST(@ErrorNumber AS VARCHAR(10));
        PRINT 'Actual line number: ' + CAST(@ErrorLine AS VARCHAR(10));

        RAISERROR(@ErrorMessage, @ErrorSeverity, @ErrorState);
END CATCH
END

I'm not too good in the area of cursors but what I need to happen is everything inside the WHILE block needs to run based on an ItemID. I can separate this script into two files….one that just updates the Assets and the other that just updates the Inventory table, and each will run as expected. It appears the minute I put a cursor around the whole thing that it hangs.

The short story is we have many inventory records with the same ItemID but with a different serial number. We've gone away from this and they now want to show just a quantity value for a given ItemID on a single Inventory record. The same can be said for Assets except they join on InventoryID with the Inventory table. So due to relationships that are setup, I have to update the Assets portion first, then do the Inventory portion last.

Any help is greatly appreciated.

Best Answer

Just from looking at this code, something that jumps out as "clearly not good" is that you start a WHILE loop but then have 2 IF statements before the BEGIN:

WHILE @@FETCH_STATUS = 0

   --Drop Temp tables if they exist.
   IF OBJECT_ID('tempdb..#TEMP') IS NOT NULL
      DROP TABLE #TEMP

   IF OBJECT_ID('tempdb..#INV_TEMP') IS NOT NULL
      DROP TABLE #INV_TEMP

   BEGIN

The effect of that should be that only the next statement is part of the WHILE and so it does an infinite loop of:

IF OBJECT_ID('tempdb..#TEMP') IS NOT NULL
      DROP TABLE #TEMP

since @@FETCH_STATUS never gets updated since no additional FETCH statement is ever reached.

Move the BEGIN to just after the WHILE @@FETCH_STATUS = 0 and before that first IF:

WHILE @@FETCH_STATUS = 0
BEGIN

   --Drop Temp tables if they exist.
   IF OBJECT_ID('tempdb..#TEMP') IS NOT NULL
      DROP TABLE #TEMP

   IF OBJECT_ID('tempdb..#INV_TEMP') IS NOT NULL
      DROP TABLE #INV_TEMP

Additionally, why do you have the BEGIN TRAN / COMMIT outside of the cursor? That effectively makes the processing of ALL ItemIDs a single operation, instead of just the statements per each @ItemID a single operation. Is this what you really want? Or were you just wanting each individual @ItemID, and all of the operations for it, to be an atomic operation, but still separate from the other ItemIDs? If wanting them to be separate (which seems to be the more likely case), then move the BEGIN TRAN to just inside of the BEGIN for the WHILE, and move the COMMIT to just before the FETCH NEXT FROM LoopCursor at the end of the loop.