0
protected void addItem_Click(object sender, EventArgs e)
{
    String CS = ConfigurationManager.ConnectionStrings["DatabaseConnectionString"].ConnectionString;
    using (SqlConnection con = new SqlConnection(CS))
    {
        string PID;
        Button oButton = (Button)sender;
        PID = oButton.CommandArgument.ToString();

        int productId = Convert.ToInt32(PID);
        Debug.Write(productId);

        string email = (string)(Session["email"]);

        SqlCommand cmd = new SqlCommand("insert into basket (productId, email) values( productId,'" + email + "')", con);

        con.Open();
        cmd.ExecuteNonQuery();
    }
}

When my query executes, I get an error

Invalid column name 'productId'

As you can see, I have converted a string into an integer variable, I have printed off the variable to check what it is returning. It does return a int as expected, but for some odd reason i can not insert into to my table. Any help would be great.

2
  • values( productId,'" + email + "') I see a big problem with that...you're inserting the value productId (which is not a valid value) not the variable. Try values(" + productId + ",'" + email + "')" then look into SQL Injection. Commented Apr 13, 2017 at 4:39
  • 1
    Never preparing SQL query in c#, as it make vulnerable for SQL Injection. Instead prepare stored procedure in database and pass parameters to it. Commented Apr 13, 2017 at 4:41

2 Answers 2

5

Hope productId is not the primary key of the basket table.

Then,instead of

 SqlCommand cmd = new SqlCommand("insert into basket (productId, email) values( productId,'" + email + "')", con);

Modify like below

 SqlCommand cmd = new SqlCommand("insert into basket (productId, email) values( @productId,@email)", con);
cmd.Parameters.Add(new SqlParameter("@productId",productId );
cmd.Parameters.Add(new SqlParameter("@email",email );

Why suggested to modify is to avoid SQLInjection attack. If you are unaware about that please go through the below link and learn it

https://en.wikipedia.org/wiki/SQL_injection

Sign up to request clarification or add additional context in comments.

2 Comments

Thanks so much it worked, can you explain why my approach was not working?
@MahirAhmed as I mentioned in my answer, you were not passing in the productId variable, productId was part of your query string meaning it was looking to fill the productId column with the value from the productId column.
1

Two issues here, number 1, and a big one, parameterize that query! You're opening yourself up to SQL injection attacks with code like that.

The second is that you're not actually passing in your productId variable, you're telling it to use the value for the productId column - which is also the column you're trying to insert into.

SqlCommand cmd = new SqlCommand("insert into basket (productId, email) values (@productId, @email)");
cmd.Parameters.AddWithValue("@productId", productId);
cmd.Parameters.AddWithValue("@email", email);

I can't stress enough how dangerous it is to dump user input into SQL that's going to be run directly on your database.

1 Comment

thanks mate, your approached worked for me and yes i'm very new to coding and need to learn about sql injections in depth, i appreciate you pointing it out to me!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.