Rollback transaction not working

Question

I have two tables tblproduct and tblproductsales. I want to see how Begin Tran, Rollback work.

There is one insert statement and update statement in the stored proc. If either one of statements fail, both should rollback. but rollback is not working even if Update statement is failing. any suggestion please?

CREATE PROCEDURE spbasam_ProductSales
@ProductId int,
@QtyNeeded int
AS
BEGIN
--Check the if you have enough stock to see
DECLARE @productavailabiltycount int
SELECT @productavailabiltycount = QtyAvailable FROM dbo.tblProduct where ProductId = @ProductId

--check to see if you have enough

If (@productavailabiltycount < @QtyNeeded)

    BEGIN
        Raiserror('Not enough stock available',16,1)

    END
Else
    BEGIN
        BEGIN TRY   
            BEGIN TRAN
            --Step 1 to reduce tblProduct table
                DECLARE @toupdate int

                UPDATE dbo.tblProduct set QtyAvailable = @productavailabiltycount - @QtyNeeded
                       WHERE ProductId = @ProductId
            -- Step 2 insert into tblProductSales table
               --First get the max count of the productSalesId
               DECLARE @maxcountid int

               SELECT @maxcountid = MAX(ProductSalesId) from dbo.tblProductSales
               INSERT INTO dbo.tblProductSales Values(@maxcountid+1 , @ProductId, @QtyNeeded)

            COMMIT TRAN     
        END TRY
        BEGIN CATCH
            Rollback Transaction

            SELECT 
                ERROR_NUMBER() as ErrorNumber,
                ERROR_MESSAGE() as ErrorMessage,
                ERROR_PROCEDURE() as ErrorProcedure,
                ERROR_STATE() as ErrorState,
                ERROR_LINE() as ErrorLine

        END CATCH       
    END
 END

 Exec spbasam_ProductSales 1,10

How do you know that the update is failing? If it doesn't produce an error then it won't rollback the transaction. — Avitus
Perhaps you should add this line right after the update statement: if @@rowcount = 0 raiserror('Update failed.'16,1), then it would kick it to the catch block and won't continue to execut the remaining statements after the update statement. — James L.
Hopefully there are also checks to make sure the quantities can only be non-negative (unless you have some sort of backorder process handling also). Also probably some concurrency issues here. If two users run this at the same time and both get back a count of 1 QtyAvailable in the first check, then both subtract a value in the later transaction, now you have -1. — Jacob H
Aside: You are getting a copy of QtyAvailable outside the transaction and then using the (possibly stale) value within the transaction. That's a recipe for a difficult to locate race condition if another process alters the value between your operations. It's all the more fun since you don't ... set QtyAvailable -= @QtyNeeded ... in the update. That would at least use the latest value, even if the if condition was no longer met. — HABO
Another aside: Creating a faux identity column (@MaxCountId + 1) is generally to be avoided. And identifying the columns in insert statements (insert ( ProductSalesId, ProductId, QuantityStolen ) values ( @OptimisticValue, @ProductId, @QtyNeeded );) makes maintenance somewhat more predictable. (What happens if the table schema changes, e.g. a column is added between existing columns?) — HABO

James L. James L. · Accepted Answer · 2018-02-06T20:25:02

If the update fails because of a SQL error, then an error will be raised and it will jump to the catch block. However, if the update fails because it the SQL simply doesn't find a record to update, then no error will be raise and the next code will continue to execute. So, you should add a check to see how many rows were updated, right after the update statement, and raise your own error if nothing was updated.

...
UPDATE dbo.tblProduct set QtyAvailable = QtyAvailable - @QtyNeeded
WHERE ProductId = @ProductId and QtyAvailable >= @QtyNeeded

if @@rowcount = 0
  raiserror('Insufficient quantity available.',16,1)
...

This will kick execution to the catch block if no records were updated.

And, per @JacobH suggestion in the comment to the original post, it would be good to NEVER allow the available quantity to go negative. So, if you add an additional check in the where clause, you could avoid going negative, and one user who orders at the same time as another won't be able to complete their order because the quantity will have decremented before their order is processed. (Good catch Jacob!)

Really, you could rewrite the whole thing to eliminate the variable @productavailabiltycount since you can simply check this inside the transaction, and only update the record if there is enough quantity available for the current transaction.

Rollback transaction not working

1 Answers