CODEWITHSUNDEEP
sqlsql-serverstored-procedures
Neil
Neil

Reputation: 25

Stored procedure not updating record

I have an issue with a stored procedure where it is updating one table, but not another, and it only happens on the odd occasion, not all the time. This is the stored procedure:

ALTER PROCEDURE [dbo].[AddTransaction]
    @BeneficiaryName VARCHAR(200),
    @Amount DECIMAL(18,2),
    @Reference VARCHAR(200),
    @Direction INT,
    @Currency INT,
    @CurrencyCulture VARCHAR(5),
    @Status INT,
    @Month INT,
    @Year INT,
    @BankAccountId INT,
    @BusinessId INT,
    @FeeType INT,
    @IncomingPaymentsPercentage DECIMAL(18,2),
    @IncomingPaymentFee DECIMAL(18,2),
    @CompletedPaymentFee DECIMAL(18,2),
    @DateProcessed DATETIME
AS
BEGIN
    
    DECLARE @oldBalance DECIMAL(18,2)
    DECLARE @newBalance DECIMAL(18,2)
    DECLARE @fee DECIMAL(18,2)
    DECLARE @amountAfterFee DECIMAL(18,2)

    BEGIN TRANSACTION
        -- SET NOCOUNT ON;
        
        -- lock table 'BankAccounts' till end of transaction
        SELECT @oldBalance = availableBalance
        FROM BankAccounts
        WITH (TABLOCKX, HOLDLOCK)
        WHERE Id = @BankAccountId

        -- set the new balance
        SET @newBalance = @oldBalance + @Amount

        UPDATE BankAccounts SET AvailableBalance = @newBalance WHERE Id = @BankAccountId

        -- insert a new transaction
        IF @FeeType = 1
        BEGIN
            IF @Direction = 1
            BEGIN
                SET @fee = @Amount * (@IncomingPaymentsPercentage / 100)
                SET @amountAfterFee = @Amount - @fee;
            END;
            ELSE
            BEGIN
                SET @fee = 0.00
                SET @amountAfterFee = @Amount
            END;
        END;
        ELSE
        BEGIN
            IF @Direction = 1
            BEGIN
                SET @amountAfterFee = @Amount - @IncomingPaymentFee
                SET @fee = @IncomingPaymentFee
            END;
            ELSE
            BEGIN
                SET @amountAfterFee = @Amount - @CompletedPaymentFee
                SET @fee = @CompletedPaymentFee
            END;
        END;

        INSERT INTO Transactions
            (BeneficiaryName, amount, Reference, Direction, Currency, CurrencyCulture, DateAdded, [Status], DateProcessed, [Month], [Year], Balance, BankAccountId, AmountAfterFee, Fee)
        VALUES
            (@BeneficiaryName, @Amount, @Reference, @Direction, @Currency, @CurrencyCulture, @DateProcessed, @Status, @DateProcessed, @Month, @Year, @newBalance, @BankAccountId, @amountAfterFee, @fee)
    
    -- now commit the transaction
    COMMIT

The insert in to the transactions table is working all the time, but on the odd occasion, the update on the BankAccounts table doesn't happen. This line here:

UPDATE BankAccounts SET AvailableBalance = @newBalance WHERE Id = @BankAccountId

I am not sure if this is anything to do with the lock, but I thought you could still update the locked table within the same transaction. For more info, there could be multiple calls to this stored procedure in the same second, probably no more than 10, but often the issue is when nothing else is accessing the BankAccount record it is updating.

Upvotes: 1

Views: 886

Answers (1)

Charlieface
Charlieface

Reputation: 72480

First off, TABLOCKX on a SELECT statement does not take an X-lock. SELECT statements never take X-locks anyway. Furthermore, it is unnecessary and extremely inefficient to lock the entire table.

Secondly, even HOLDLOCK is not enough here, because if no data has been modified, the lock will not be taken. You must use UPDLOCK for this to work as expected.

To be honest, you don't actually need the SELECT anyway, as the whole thing can be done in one atomic statement:

I note that @oldBalance isn't actually used.

UPDATE BankAccounts WITH (HOLDLOCK)
SET AvailableBalance +=  @amount,
    @newBalance = AvailableBalance + @amount
WHERE Id = @BankAccountId;

You can even combine the Transaction insert by using an OUTPUT clause, this enables you to entirely remove the BEGIN TRANSACTION/COMMIT

ALTER PROCEDURE [dbo].[AddTransaction]
    @BeneficiaryName VARCHAR(200),
    @Amount DECIMAL(18,2),
    @Reference VARCHAR(200),
    @Direction INT,
    @Currency INT,
    @CurrencyCulture VARCHAR(5),
    @Status INT,
    @Month INT,
    @Year INT,
    @BankAccountId INT,
    @BusinessId INT,
    @FeeType INT,
    @IncomingPaymentsPercentage DECIMAL(18,2),
    @IncomingPaymentFee DECIMAL(18,2),
    @CompletedPaymentFee DECIMAL(18,2),
    @DateProcessed DATETIME
AS

SET NOCOUNT ON;
    
DECLARE @fee DECIMAL(18,2);
DECLARE @amountAfterFee DECIMAL(18,2);

SET @fee = CASE WHEN @FeeType = 1 THEN
    CASE WHEN @Direction = 1
        THEN @Amount * (@IncomingPaymentsPercentage / 100)
        ELSE 0.0 END
    ELSE
    CASE WHEN @Direction = 1
        THEN @IncomingPaymentFee
        ELSE @CompletedPaymentFee END
    END;
SET @amountAfterFee = @Amount - @fee;

UPDATE BankAccounts WITH (HOLDLOCK)
SET AvailableBalance += @amount

OUTPUT
    @BeneficiaryName, @Amount, @Reference, @Direction, @Currency,
    @CurrencyCulture, @DateProcessed, @Status, @DateProcessed,
    @Month, @Year, inserted.AvailableBalance, @BankAccountId, @amountAfterFee, @fee

INTO Transactions
   (BeneficiaryName, amount, Reference, Direction, Currency,
    CurrencyCulture, DateAdded, [Status], DateProcessed, [Month], [Year],
    Balance, BankAccountId, AmountAfterFee, Fee)

WHERE Id = @BankAccountId;

GO

Upvotes: 1

Related Questions