-3

Why this code is not deleting row and show error message

 protected void ImageButton1_Click(object sender, ImageClickEventArgs e)
    {
        try
        {
            String connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
            SqlConnection con = new SqlConnection(connectionString);
            SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid ('"+cids.Text+"')",con);
            con.Open();
            cmd.ExecuteNonQuery();
            Response.Redirect("done.aspx");
            con.Close();
        }
        catch (SqlException)
        { Label1.Text = "error";
        }
    }
Nagaraj S
  • 13,316
  • 6
  • 32
  • 53
user3359284
  • 21
  • 1
  • 1
  • 2
  • 1
    What error message is it showing? (Hint: change `catch(SqlException)` to `catch(SqlException e)` and post the value of `e`) – crthompson Feb 27 '14 at 07:17
  • I suspect your SQL is a little rusty. Perhaps you should consult the manual: https://dev.mysql.com/doc/refman/5.7/en/delete.html – Tieson T. Feb 27 '14 at 07:20

4 Answers4

4

USE SQL PARAMETERS, YOUR CODE IS TOTALLY UNSAFE, FIX IT!

Did you mean to use the SQL IN clause? That would be "WHERE cid IN (...)", or just single value with "WHERE cid = ..."

See Parameterize an SQL IN clause to use parameters with IN clause

Community
  • 1
  • 1
TFD
  • 23,890
  • 2
  • 34
  • 51
  • now using this SqlCommand cmd = new SqlCommand("DELETE from [course] where cid IN ('" + cids.Text + "')", con); – user3359284 Feb 27 '14 at 07:36
  • @user3359284 the point TFD is making is that _any_ query you build strictly by creating a string has the potential to cause problems, including the potential to have very unwanted queries injected into your database through this insecure hole you create. Imagine for example that `cids.Text` is `"); drop table course"`... your query is not likely to please you! Using **proper** parameterized query creation avoids this. – mah Feb 27 '14 at 15:33
1
SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid ('"+cids.Text+"')",con);

Should be:

SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid IN ('"+cids.Text+"')",con);

Notice the "in" between cid and (

The assumption is that you should use in because of the parens. But if you have only one value you are wanting to compare to then = should do the trick.

Also, as @TFD notes, you should use parameterized SQL. This code will allow for SQL Injection attacks.

crthompson
  • 15,653
  • 6
  • 58
  • 80
1

Ok, did you ever read the SQL syntax somewhere? Or are you just throwing random stuff at a wall and hoping it sticks?

cid ('"+cids.Text+"')",

Let's ignore the fact that this is the best way to get fired - anyone can execute ANY RANDOM SQL HE WANTS in your database. Bad news - happens when you do not check your input (cids.Text.

But brutally sppeaking, if I have 55 in the text box, that translates into:

cid ('55')

Ok, where does that come from?

() indiate IN clause, but there is no IN.

SQL syntax says quality are = - that would be cid = 55

THen a number is not a string (no need for '55', it is 55).

Correct would be

cid = 55

Please, a one minute check of the documentation is sometimes ncice to learn a language. Helps to not throw random statements into a compiler hoping they are correct.

TomTom
  • 61,059
  • 10
  • 88
  • 148
0

you forget to use =

SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid ('"+cids.Text+"')",con);

This line shoud be

SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid ='"+cids.Text+"'",con);

or use WHERE cid IN (...)

SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid IN ('"+cids.Text+"')",con);

protected void ImageButton1_Click(object sender, ImageClickEventArgs e)
    {
        try
        {
            String connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
            SqlConnection con = new SqlConnection(connectionString);
            SqlCommand cmd=new SqlCommand("DELETE  from [course] where cid IN ('"+cids.Text+"')",con);
            con.Open();
            cmd.ExecuteNonQuery();
            Response.Redirect("done.aspx");
            con.Close();
        }
        catch (SqlException ex)
        { 
         Label1.Text = "error";
        }
    }
Nagaraj S
  • 13,316
  • 6
  • 32
  • 53