Updating mysql database cause a error

Question

I was trying to update my mysql database but it didn't work and showed me error.

The code :

string constring = "datasource=localhost;port=3306;username=root;password=root";
string Query = "update  database.check set  namethestore = '" + this.textBox65.Text + "' , checkername= '" + this.textBox66.Text + "' ,  where namethestore = '" + this.textBox65.Text + "'  ;";
MySqlConnection conDataBase = new MySqlConnection(constring);
MySqlCommand cmdDataBase = new MySqlCommand(Query, conDataBase);
MySqlDataReader myReader;
try
{
   conDataBase.Open();
   myReader = cmdDataBase.ExecuteReader();
   MessageBox.Show("saved");

   while (myReader.Read())
   {
   }

You are exposed to SQL Injection. Look at this: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php — Horaciux, Jul 05 '14 at 14:51
@user3806048 You build an `UPDATE` statement - and then try a reader on this statement. I think you should use the `ExecuteNonQuery()` method. — VMai, Jul 05 '14 at 14:52

score 1 · Answer 1 · answered Jul 05 '14 at 14:56

There are so many things wrong on that code.

You should use ExecuteNonQuery to an update statement
You must use parameterised query to protect your code from SQL Injection
AS Rahul said, you have an extra ,
I would like to know if you are closing the connection at the end.

Rahul · Accepted Answer · 2014-07-05T15:12:39.023

0

You have a extra , in your UPDATE statement before WHERE condition as below

update  database.check set  namethestore = '" + this.textBox65.Text + "' , 
checkername= '" + this.textBox66.Text + "' ,  where ...;

                                           <--Here

As other's have already commented, your code is vulnerable to SQL Injection. Try using a parametrized query rather.

Also, since you are performing a DML operation, no need of a DataReader. You can either use ExecuteNonQuery or ExecuteScalar.

For your reference:

string constring = "datasource=localhost;port=3306;username=root;password=root";
string Query = "update  database.check set  namethestore = 
@namethestore,checkername=@checkername where namethestore = @namethestore";
using(MySqlConnection conDataBase = new MySqlConnection(constring))
{
  using(MySqlCommand cmdDataBase = new MySqlCommand(Query, conDataBase))
   {
     conDataBase.Open();
     cmdDataBase.CommandText = Query;
     cmdDataBase.Parameters.Add("@namethestore", this.textBox65.Text);
     cmdDataBase.Parameters.Add("@checkername", this.textBox66.Text);

     cmdDataBase.ExecuteNonQuery();
   }
}

edited Jul 05 '14 at 15:12

answered Jul 05 '14 at 14:51

Rahul

76,197
13
71
125

what do you mean by @namethstore ? , do you mean the name of the textbox like texbox1.txt ? – user3806048 Jul 05 '14 at 16:00
@user3806048 These are placeholders for your values. – VMai Jul 05 '14 at 16:23
@user3806048, No those are parameters which going to hold the value of textbox. You pass those parameters to your SQL query. This avoids SQL injection attack. Read more in MSDN http://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlcommand.parameters%28v=vs.110%29.aspx. – Rahul Jul 05 '14 at 17:24
i did as you asked but i still go error , my code : string Query = "update database.check set namethestore = '" + this.textBox65.Text + "' , checkername= '" + this.textBox66.Text + "' where namethestore = '" + this.textBox65.Text + "' :"; using(MySqlConnection conDataBase = new MySqlConnection(constring)) { – user3806048 Jul 05 '14 at 17:28
using(MySqlCommand cmdDataBase = new MySqlCommand(Query, conDataBase)) { conDataBase.Open(); cmdDataBase.CommandText = Query; cmdDataBase.Parameters.Add("+ this.textBox65.Text +", this.textBox65.Text); cmdDataBase.Parameters.Add(" + this.textBox66.Text +", this.textBox66.Text); cmdDataBase.ExecuteNonQuery(); } – user3806048 Jul 05 '14 at 17:29
@user3806048, that's not at all what I suggested in my answer. You don't use parameter that way. I have posted the code in my answer. If needed you can directly copy/paste it and use it. I have provided MSDN doc link in another comment. Read the document to understand command parameter better before you use it. – Rahul Jul 05 '14 at 17:30

Soner Gönül · Answer 3 · 2014-07-05T15:08:19.353

ExecuteReader returns a MySqlDataReader which contains the result of your MySqlCommand. Since you use INSERT statement, there is no data your statement that return. Your statement is just insert a value from your database.

That's why you should use ExecuteNonQuery. It just executes your statement and for INSERT statement, it returns that the number of rows affected.

And you should always use parameterized queries. This kind of string concatenations are open for SQL Injection attacks.

Also use using statement to dispose your database connections.

And you have an extra comma (,) before your WHERE condition.

using(MySqlConnection conn = new MySqlConnection(constring))
using(MySqlCommand cmd = conn.CreateCommand())
{
    cmd.CommandText = "update database.check set namethestore = @namethestore, checkername= @checkername where namethestore = @namethestore";
    cmd.Parameters.AddWithValue(@namethestore, this.textBox66.Text);
    cmd.Parameters.AddWithValue(@checkername, this.textBox65.Text);
    conn.Open()
    if(cmd.ExecuteNonQuery() > 0)
       MessageBox.Show("saved");
}

I used AddWithValue in my example because I don't know your column types but you don't use it. Use SqlParameterCollection.Add method instead.

Read http://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/

Updating mysql database cause a error

3 Answers3