Click here to Skip to main content
15,917,971 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
Fatal error encountered during command execution.

What I have tried:

C#
private void dataGridViewPayment_CellContentClick(object sender, DataGridViewCellEventArgs e)
       {
           con.Close();
           cmd = new MySqlCommand("UPDATE payment SET ornumber = @ornumber,datepayment = @datepayment,amountpaid = @amountpaid,penalty = @penalty,totalamount = @totalamount,cashtendered = @cashtendered,CHANGED = @changed WHERE contractid =" + contractid + ";", con.con);
           con.Open();
           reader = cmd.ExecuteReader();

           cmd.Parameters.AddWithValue("@ornumber", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[2].Value.ToString()));
           cmd.Parameters.AddWithValue("@datepayment", DateTime.Parse(dataGridViewPayment.SelectedRows[0].Cells[3].Value.ToString()));
           cmd.Parameters.AddWithValue("@amountpaid", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[4].Value.ToString()));
           cmd.Parameters.AddWithValue("@penalty", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[7].Value.ToString()));
           cmd.Parameters.AddWithValue("@totalamount", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[8].Value.ToString()));
           cmd.Parameters.AddWithValue("@cashtendered", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[9].Value.ToString()));
           cmd.Parameters.AddWithValue("@changed", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[10].Value.ToString()));

       }
Posted
Updated 11-Oct-22 1:33am
Comments
Richard MacCutchan 11-Oct-22 3:51am    
Where exactly did this error occur?
PIEBALDconsult 11-Oct-22 9:15am    
Please don't use ToString and then immediately use Parse -- just cast the value.

So many problems in such a small code sample!

Quote:
C#
new MySqlCommand("UPDATE payment SET ornumber = @ornumber,datepayment = @datepayment,amountpaid = @amountpaid,penalty = @penalty,totalamount = @totalamount,cashtendered = @cashtendered,CHANGED = @changed WHERE contractid =" + contractid + ";", con.con)
You know you need to use parameters to avoid SQL Injection[^]. You sort-of know how to use them. And yet you still chose to concatenate a parameter value directly into your query at the last, leaving your code wide-open to this critical vulnerability.
Quote:
C#
con.Open();
reader = cmd.ExecuteReader();

cmd.Parameters.AddWithValue("@ornumber", Int32.Parse(dataGridViewPayment.SelectedRows[0].Cells[2].Value.ToString()));
And this is why I said you "sort-of" know how to use them - you've tried to execute the command before adding the parameters to it.

You need to add the parameters first; only then can you execute the command.
Quote:
C#
cmd = new MySqlCommand("UPDATE ...", con.con);
con.Open();
reader = cmd.ExecuteReader();
The ExecuteReader method is meant to be used with commands which return data - typically a SELECT command. For an UPDATE command, you should use ExecuteNonQuery instead.
Quote:
C#
con.Close();
cmd = new MySqlCommand(...);
con.Open();
reader = cmd.ExecuteReader();
You're storing your connection, command, and datareader objects in class-level fields. That's a great way to introduce memory leaks and data corruption!

Instead, create the objects when you need them, and wrap them in a using block to ensure they're always disposed of properly. I'd suggest creating a function to create your connection object, so that you only need to configure it in one place. For example:
C#
private MySqlConnection CreateConnection()
{
    return new MySqlConnection(ConfigurationManager.ConnectionStrings["MyConnectionString"].ConnectionString);
}

private void dataGridViewPayment_CellContentClick(object sender, DataGridViewCellEventArgs e)
{
    if (dataGridViewPayment.SelectedRows.Count == 0)
    {
        // TODO: Tell the user to select a row.
        return;
    }
    
    var row = dataGridViewPayment.SelectedRows[0];
    
    using (MySqlConnection con = CreateConnection())
    using (MySqlCommand cmd = new MySqlCommand("UPDATE payment SET ornumber = @ornumber, datepayment = @datepayment, amountpaid = @amountpaid, penalty = @penalty, totalamount = @totalamount, cashtendered = @cashtendered, CHANGED = @changed WHERE contractid = @contractid;", con))
    {
       cmd.Parameters.AddWithValue("@ornumber", Int32.Parse(row.Cells[2].Value.ToString()));
       cmd.Parameters.AddWithValue("@datepayment", DateTime.Parse(row.Cells[3].Value.ToString()));
       cmd.Parameters.AddWithValue("@amountpaid", Int32.Parse(row.Cells[4].Value.ToString()));
       cmd.Parameters.AddWithValue("@penalty", Int32.Parse(row.Cells[7].Value.ToString()));
       cmd.Parameters.AddWithValue("@totalamount", Int32.Parse(row.Cells[8].Value.ToString()));
       cmd.Parameters.AddWithValue("@cashtendered", Int32.Parse(row.Cells[9].Value.ToString()));
       cmd.Parameters.AddWithValue("@changed", Int32.Parse(row.Cells[10].Value.ToString()));
       cmd.Parameters.AddWithValue("@contractid", contractid);
       con.Open();
       cmd.ExecuteNonQuery();
   }
}
 
Share this answer
 
v2
Comments
PIEBALDconsult 11-Oct-22 9:14am    
"The ExecuteReader method is meant to be used with commands which return data" -- not really. ExecuteReader can execute any command, or many commands. When you call ExecuteNonQuery or ExecuteScalar, they call ExecuteReader and hide the details.
Richard Deeming 11-Oct-22 9:30am    
Not always; at least for SQL Server, context connections and parameterless commands don't go via RunExecuteReader. The MySQL provider may have different behaviour.

But even if you can guarantee that your provider does internally use the ExecuteReader path, there's no benefit to using ExecuteReader for commands which don't return any data. Particularly not if you're going to leave a dangling reader stored in a field of your class! :)
PIEBALDconsult 11-Oct-22 9:41am    
Well, sure, never leave your reader dangling.
All's I know is that at times, when a call to ExecuteNonQuery or ExecuteScalar fails, the stack trace shows that ExecuteReader was called, and _to_me_ it makes sense that calling ExecuteReader in the background is better than implementing a bunch of similar but different methods.
Still, yes, I prefer to call ExecuteReader or ExecuteScalar when I know that's what I want.
Richard MacCutchan 11-Oct-22 11:16am    
And all those calls to Int32.Parse() have the potential to crash the application.
Richard Deeming 11-Oct-22 11:17am    
Indeed, but that's a problem for another day. :)
Without your code running in context, and access to your data we can't tell at all what is going on, or what might possibly be causing the problem.

So, it's going to be up to you.
Fortunately, you have a tool available to you which will help you find out what is going on: the debugger. If you don't know how to use it then a quick Google for "Visual Studio debugger" should give you the info you need.

Put a breakpoint on the first line in the function, and run your code through the debugger. Then look at your code, and at your data and work out what should happen manually. Then single step each line checking that what you expected to happen is exactly what did. When it isn't, that's when you have a problem, and you can back-track (or run it again and look more closely) to find out why.

Sorry, but we can't do that for you - time for you to learn a new (and very, very useful) skill: debugging!
 
Share this answer
 
Comments
Marc Chester Ferrer 11-Oct-22 22:24pm    
thank you OriginalGriff

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900