How to solve "there is already an open data reader with this command" error?

Question

My code goes like this

SqlDataReader read=command1.ExecuteReader();
while(reader.Read())
{
    // based on each data read from a table1 I want to run an update query on table1 itself
    SqlCommand command2= new SqlCommand();//command with update query and connection is the same as command1

    command2.ExecuteNonQuery();
}//end of while

The error occurs at command2.ExecuteNonQuery().

Is there a way to go around this problem?

I am using the same connection for both commands.

Edit,full code is given below. I read data from shoporder table to a data sqlreader object. Then ago through each record and update LaborCost colmn using update query.

private void button1_Click(object sender, EventArgs e)
{
    string ConnectionString=@"Data Source=DESKTOP-KM9K7OP\SQLEXPRESS;Initial Catalog=TheSinkSQLVersion;Integrated Security=True";

    SqlConnection connection1 = new SqlConnection();
    SqlConnection connection2= new SqlConnection();// As suggested by CAIUS JARD
    connection2.ConnectionString = ConnectionString;
    connection1.ConnectionString = ConnectionString;
    connection1.Open();
    connection2.Open();
    SqlCommand command1 = new SqlCommand();
    SqlCommand command2;
    string SqlString = "Select ShopOrderNo, LaborCost, TireRepairCost From shoporder Where TireRepairCost > '0'";
    command1.Connection = connection1;
    command1.CommandText = SqlString;
            
    using (SqlDataReader reader = command1.ExecuteReader())
    {
        while (reader.Read())
        {
            command2 = new SqlCommand();
            command2.Connection = connection2;
            decimal newLaborCost = (decimal)reader["LaborCost"] + (decimal)reader["TireRepairCost"];
            decimal tireRepairCost = 0.0m;//set tire repair cost to zero

            string sqlString2 = "Update shoporder Set LaborCost= @LaborCost, TireRepairCost = @TireRepairCost Where ShopOrderNo ='" + reader["ShopOrderNo"] + "'";
            command2.CommandText = sqlString2;
            SqlParameter param = new SqlParameter("@LaborCost", newLaborCost); command2.Parameters.Add(param);
            param = new SqlParameter("@TireRepairCost", tireRepairCost); command2.Parameters.Add(param);
            command2.ExecuteNonQuery();
            command2.Dispose();
        }//End While
        reader.Close();
    }//end using reader
                
    connection1.Close();
    connection2.Close();            
} //end btnClick

This might help https://stackoverflow.com/questions/6062192/there-is-already-an-open-datareader-associated-with-this-command-which-must-be-c — Hari, Feb 15 '22 at 12:24
Apologies - i just looked in the code, and couldn't see any code that assigned the connection - I overlooked the part where you said outside the code that connection re-use was happening. Maybe I'm going senile, but I'm sure I've had the error more helpfully say "there is already an open...associated with this *connection*" in the past.. — Caius Jard, Feb 15 '22 at 12:40
@CaiusJard, I'll try and getback to you, what about stackoverflow.com/questions/6062192/… — , Feb 15 '22 at 12:41
Incidentally, this "run some query and then execute updates RBAR style is usually forged from thinking about the problem in a row-by-row fashion rather than a set fashion. It should be possible to rewrite it so it doesn't work this way but we need info on what you're doing — Caius Jard, Feb 15 '22 at 12:42
*what about the duplicate* - well.. There are a lot of options there; I'm not convinced that enabling MARS or downloading a billion rows into the client is the right thing to do, but it would help to know what you're actually doing.. If e.g. youre doing this update piecemeal because you don't know how to do it in a one hit UPDATE, we should change that. If there is data coming from e.g. an API call that C# only can do, then we should download some block of data, API it all and then UPDATE it.. The answer could be quite contextual; post more code — Caius Jard, Feb 15 '22 at 12:43
@CaiusJard, can you elaborate on "youre doing this update piecemeal because you don't know how to do it in a one hit UPDATE", — , Feb 15 '22 at 13:02
You go first; post the full code you're executing and I'll suggest something — Caius Jard, Feb 15 '22 at 13:08
@CaiusJard, I have added the full code, thank you for the help — , Feb 15 '22 at 13:38

Caius Jard · Accepted Answer · 2022-02-15T13:52:07.867

As coded, the entire operation can be collapsed to just executing an SQL of:

UPDATE shoporder 
SET LaborCost = LaborCost + TireRepairCost, TireRepairCost = 0 
WHERE TireRepairCost > 0

Minimally that would look like:

private void button1_Click(object sender, EventArgs e)
{
  using var c = new SqlCommand("UPDATE shoporder SET LaborCost = LaborCost + TireRepairCost, TireRepairCost = 0 WHERE TireRepairCost > 0", ConnectionString);
  c.Connection.Open();
  c.ExecuteNonQuery();
}

Footnote

If you're going to be doing a lot of database work like this, switching to using Dapper will help you out. Dapper code would be:

private async void button1_Click(object sender, EventArgs e)
{
  using var c = new SqlConnection(ConnectionString);
  await c.ExecuteAsync("UPDATE shoporder SET LaborCost = LaborCost + TireRepairCost, TireRepairCost = 0 WHERE TireRepairCost > 0");
}

..but the real magic of Dapper is where you want to run DB queries and get objects:

using var c = new SqlConnection(ConnectionString);
var orders = await c.QueryAsync<ShopOrder>("SELECT * FROM shopOrders WHERE TireRepairCost > @cost", new{ cost = 0 }).ToList();

Just those two lines would give you a list of ShopOrder collection objects to do stuff with. Doing it "the long way" would be more like:

using var c = new SqlCommand("SELECT * FROM shopOrders WHERE TireRepairCost > @cost", ConnectionString);
c.Parameters.AddWithValue("@cost", 0); //btw, google 'addwithvalue is evil'
c.Connection.Open();
var r = c.ExecuteReader();
var orders = new List<ShopOrder>();
while(r.Read()){
  orders.Add(new ShopOrder {
    Id = r.GetInteger("Id"),
    LaborCost = r.GetInteger("LaborCost"),
    TireCost = r.GetInteger("TireCost"),
  });
}

Painful..

Hmmmm, This is why you should ask what you dont know! Thank you again! — , Feb 15 '22 at 13:49
No problem! I added a bit on Dapper too, the technology that runs stackoverflow; it could speed up your life a lot! — Caius Jard, Feb 15 '22 at 13:57
We all started there! :) (And then eventually when someone like Sam Saffron/Mark Gravell think "gee, this is repetitive and boring, I'll write some code to remove the boring part", you get Dapper. Everything we do is that, on some level. Assembly language/machine code is really boring, so we have IL, but that's repetitive and boring, so we have C#, but that's R&B, so we have Dapper, EF, the windows Forms designer (look at all the R&B code it writes in Form1.Designer.cs).. Funny how eventually even the time savers we wrote to save ourselves from R&B become R&B and need a time saver writing.. :D — Caius Jard, Feb 15 '22 at 15:59

How to solve "there is already an open data reader with this command" error?

1 Answers1

Footnote