views:

516

answers:

5

Replaces Question: http://stackoverflow.com/questions/184096/update-multiple-rows-into-sql-table

Here's a Code Snippet to update an exam results set. DB structure is as given, but I can submit Stored Procedures for inclusion (Which are a pain to modify, so I save that until the end.)

The question: Is there a better way using SQL server v 2005.,net 2.0 ?

string update = @"UPDATE dbo.STUDENTAnswers 
                              SET ANSWER=@answer
                              WHERE StudentID =@ID and QuestionNum =@qnum";
            SqlCommand updateCommand = new SqlCommand( update, conn );
            conn.Open();

            string uid = Session["uid"].ToString();
            for (int i= tempStart; i <= tempEnd; i++)
            {
                updateCommand.Parameters.Clear();
                updateCommand.Parameters.AddWithValue("@ID",uid);
                updateCommand.Parameters.AddWithValue("@qnum",i);
                updateCommand.Parameters.AddWithValue("@answer", Request.Form[i.ToString()]);
                try
                {
                    updateCommand.ExecuteNonQuery();
                }
                catch { }
            }
A: 

emit a single update that goes against a values table:

UPDATE s SET ANSWER=a FROM dbo.STUDENTAnswers s JOIN (
  SELECT 1 as q, 'answer1' as a
  UNION ALL SELECT 2, 'answer2' -- etc...
) x ON s.QuestionNum=x.q AND StudentID=@ID

so you just put this together like this:

using(SqlCommand updateCommand = new SqlCommand()) {
  updateCommand.CommandType = CommandType.Text;
  updateCommand.Connection = conn;
  if (cn.State != ConnectionState.Open) conn.Open();

  StringBuilder sb = new StringBuilder("UPDATE s SET ANSWER=a FROM dbo.STUDENTAnswers s JOIN (");
  string fmt = "SELECT {0} as q, @A{0} as a";
  for(int i=tempStart; i<tempEnd; i++) {
    sb.AppendFormat(fmt, i);
    fmt=" UNION ALL SELECT {0},@A{0}";
    updateCommand.Parameters.AddWithValue("@A"+i.ToString(), Request.Form[i.ToString()]);
  }
  sb.Append(") x ON s.QuestionNum=x.q AND StudentID=@ID");
  updateCommand.CommandText = sb.ToString();
  updateCommand.Parameters.AddWithValue("@ID", uid);
  updateCommand.ExecuteNonQuery();
}

This has the advantages of being an all other nothing operation (like if you'd wrapped several updates in a transaction) and will run faster since:

  • The table and associated indexes are looked at/updated once
  • You only pay for the latency between your application and the database server once, rather than on each update
Hafthor
A: 

An issue I see is when you are opening your connection.

I would at least before every update call the open and then close the connection after the update.

If your loop takes time to execute you will have your connection open for a long time.

It is a good rule to never open your command until you need it.

David Basarab
Loop runs about 30 times per postback.My thinking was that I'd save opening and closing 30 times. Am I wrong here?
chris
I agree, I'd rather have an open connection for however long the loop needs to execute than open and close the connection N times.
palmsey
A: 

You can bulk insert using OpenXML. Create an xml document containing all your questions and answers and use that to insert the values.

Edit: If you stick with your current solution, I would at least wrap your SqlConnection and SqlCommand in a using block to make sure they get disposed.

palmsey
+2  A: 

For 30 updates I think you're on the right track, although the comment about the need for a using around updateCommand is correct.

We've found the best performing way to do bulk updates (>100 rows) is via the SqlBulkCopy class to a temporary table followed by a stored procedure call to populate the live table.

mancaus
+4  A: 

A few things stand out:

  • You don't show where the SqlConnection is instantiated, so it's not clear that you're disposing it properly.

  • You shouldn't be swallowing exceptions in the loop - better to handle them in a top level exception handler.

  • You're instantiating new parameters on each iteration through the loop - you could just reuse the parameters.

Putting this together it could look something like the following (if you don't want to use a transaction, i.e. don't care if some but not all updates succeed):

using (SqlConnection conn = new SqlConnection(connectionString))
{
    conn.Open();
    using (SqlCommand updateCommand = new SqlCommand(update, conn))
    {
        string uid = Session["uid"].ToString();
        updateCommand.Parameters.AddWithValue("@ID", uid);
        updateCommand.Parameters.AddWithValue("@qnum", i);
        updateCommand.Parameters.Add("@answer", System.Data.SqlDbType.VarChar);
        for (int i = tempStart; i <= tempEnd; i++)
        {
            updateCommand.Parameters["@answer"] = Request.Form[i.ToString()];
            updateCommand.ExecuteNonQuery();
        }
    }
}

Or to use a transaction to ensure all or nothing:

using (SqlConnection conn = new SqlConnection(connectionString))
{
    conn.Open();
    using (SqlTransaction transaction = conn.BeginTransaction())
    {
        using (SqlCommand updateCommand = new SqlCommand(update, conn, transaction))
        {
            string uid = Session["uid"].ToString();
            updateCommand.Parameters.AddWithValue("@ID", uid);
            updateCommand.Parameters.AddWithValue("@qnum", i);
            updateCommand.Parameters.Add("@answer", System.Data.SqlDbType.VarChar);
            for (int i = tempStart; i <= tempEnd; i++)
            {
                updateCommand.Parameters["@answer"] = Request.Form[i.ToString()];
                updateCommand.ExecuteNonQuery();
            }
            transaction.Commit();
        }
    } // Transaction will be disposed and rolled back here if an exception is thrown
}

Finally, another problem is that you are mixing UI code (e.g. Request.Form) with data access code. It would be more modular and testable to separate these - e.g. by splitting your application into UI, Business Logic and Data Access layers.

Joe
I think handling exceptions in the loop or not should depend on business rules. Dealing with exceptions outside the loop means you either stop processing at the item with the error or rollback the transaction, but there may be cases where you want to skip the error row and continue processing.
palmsey
-- Logging code was deleted.
chris
If you want to skip an error row and continue processing other rows, then you would probably want to catch SqlException, and inspect the SqlErrors property for some specific errors.
Joe