CODEWITHSUNDEEP
c#sqlstored-proceduresatomic
NealR
NealR

Reputation: 10709

SQL atomic transaction not being atomic

Below is the code for two calls to two stored procedures that I'm working on. I thought I had this set up to either do an all or nothing atomic transaction, however if there is an error in the second stored procedure call the first will still execute. I'm still new to writing these in C#, so any help would be appreciated. Thx!

I should also mention something else this is doing that I find odd: If an exception is thrown and the program goes into the "catch" block, it will still run the code in the "finally" block. My understanding was that if an exception is thrown, the code in the "catch" block is all that will be executed.

EDIT Thanks to responses below I've fixed the catch/finally confusion and added an ExecuteNonQuery() call so the first stored procedure gets called as well. However, the first stored procedure, when it is called, needs to be called an executed first before the second stored procedure can do it's work. Is this something that can be accomplished in an atomic transaction or do they have to be run separately? 

try
{
    cm = Dts.Connections["serverName"];
    sqlConn = (SqlConnection)cm.AcquireConnection(Dts.Transaction);
    sqlTrans = sqlConn.BeginTransaction("QueueUpdates");                                


    if (dummyIndicator.Equals("Y"))
    {
        string temp = retrievedMessage.Substring(203, 17);
        int newNumber = (int)(long.Parse(temp) / 777);

        SqlParameter newNum = new SqlParameter("@newNum", num.Value);
        SqlParameter oldNum = new SqlParameter("@oldNum", newNumber);

        sqlComm = new SqlCommand("DB.dbo.sp_UpdateNumber", sqlConn, sqlTrans);
        sqlComm.CommandType = CommandType.StoredProcedure;
        sqlComm.Parameters.Add(newNum);
        sqlComm.Parameters.Add(oldNum);
        sqlComm.Transaction = sqlTrans;
        sqlComm.ExecuteNonQuery();

    }

    //Update records according to queue messages
    sqlComm = new SqlCommand("DB.dbo.sp_AgentIdAprCheck", sqlConn, sqlTrans);
    sqlComm.CommandType = CommandType.StoredProcedure;
    sqlComm.Parameters.Add(num);
    sqlComm.Parameters.Add(addOrUpdate);
    sqlComm.Parameters.Add(companyCode);
    sqlComm.Parameters.Add(agentID);
    sqlComm.Parameters.Add(firstName);
    sqlComm.Parameters.Add(lastName);
    sqlComm.Parameters.Add(suffix);
    sqlComm.Parameters.Add(taxIdType);
    sqlComm.Parameters.Add(entityType);
    sqlComm.Parameters.Add(corporateName);
    sqlComm.Parameters.Add(outNewNumber);
    sqlComm.Parameters.Add(outCurrentNumber);
    sqlComm.Parameters.Add(outOperator);
    sqlComm.Parameters.Add(outDate);
    sqlComm.Parameters.Add(returnVal);
    sqlComm.ExecuteNonQuery();

    sqlTrans.Commit();

    if (addOrUpd.Equals("ADD")){recordsAdded++;}
    else{recordsUpdated++;}

}
catch (Exception ex)
{
    _sqlDataErrors++;
    swLog.WriteLine("Message not updated: " + retrievedMessage);
    swLog.WriteLine("Error: " + ex.ToString());
    sqlTrans.Rollback();
}
finally
{                                
    cm.ReleaseConnection(sqlConn);
}

Upvotes: 2

Views: 953

Answers (3)

Konstantin Chernov
Konstantin Chernov

Reputation: 1936

First, why do you emphasis that transaction is atomic? Transaction implies smth that is always atomic, any transaction gurantees that either all of it blocks are commited or not a single block is commited.

Considering the usage of transaction in your case - it depends on your domain logic. Choice is rather simple :

  • If it's posssible to run only first sp without second - don't use single transaction.
  • If it's not possible - use it.

So if you'd provide further explanations of your domain rules and logic, we can say what scenario suites you best.

UPD Ok, after you've explained it a bit, I see no problem - for sure you can implement it that particular way and get the expected behaviour, so that commands from your both sp would be handed by single transaction and act as one. I would like also to point out possible improvement of the code.

  1. You can make exception handling more specific. Right now you are rolling back when anything goes wrong. For example - if Dts.Connections["serverName"]; is null, you'd get NullReferenceException before sqlTrans is initialized, and then in catch block you'd get another NullReferenceException,dealing with sqlTrans, which is null

  2. You can split your code into database specific and everything else, just to make sure that your Exception in that block is always exception from database layer. I mean do stuff like

    string temp = retrievedMessage.Substring(203, 17);

    int newNumber = (int)(long.Parse(temp) / 777);

somewhere else, because it's potentially dangerous and can throw an exception.

Upvotes: 1

ata
ata

Reputation: 9011

Finally block is executed no matter what (either exception occurs or not) Control is always passed to the finally block regardless of how the try block exits.

Move the transaction commit call from the finally block to after ExecuteQuery call. Also you are creating an sql command object in if block for first stored procedure and saving it in sqlComm but then outside the if block you are creating another sql command object for second stored procedure and saving it in same sqlComm object. It seems like your first stored procedure is never executed. You might have to change the logic there.

Upvotes: 0

Lucero
Lucero

Reputation: 60276

  1. You don't seem to actually execute the first SP, so my guess is the problem in the second one may be due to that?

  2. The purpose of finally is to always be executed, no matter how the code block ends (normal exit, exceptions, return statements etc. all end up executing the finally). It should therefore be used for clearing up resources, and not the catch. Move the Commit() call to be the last line before the end of the try block, and remove the ReleaseConnection from the catch block, then things should start to work as expected.

Upvotes: 0

Related Questions