Why is my SQL code in C# not working?

Question

I wrote a SQL command to save some items in my database. But when I run it, it gives an error message:

And here is my code:

public void Opslaan(string titel, string rVoornaam, string rAchternaam, decimal beoordeling, string a1Voornaam, string a1Achternaam, string a2Voornaam, string a2Achternaam, string a3Voornaam, string a3Achternaam)
    {
        if (beoordelingBest < beoordeling)
        {
            titelBest = titel;
            beoordelingBest = beoordeling;
        }
        string queryString = "INSERT INTO Films (titel, beoordeling) VALUES('" + titel + "', " + beoordeling + ");" +
                             "INSERT INTO Acteurs (voornaam, achternaam, FilmID) VALUES('" + a1Voornaam + "' , '" + a1Achternaam + "', (SELECT FilmID from Films where titel = '" + titel + "'));" +
                             "INSERT INTO Acteurs (voornaam, achternaam, FilmID) VALUES('" + a2Voornaam + "' , '" + a2Achternaam + "', (SELECT FilmID from Films where titel = '" + titel + "'));" +
                             "INSERT INTO Acteurs (voornaam, achternaam, FilmID) VALUES('" + a3Voornaam + "' , '" + a3Achternaam + "', (SELECT FilmID from Films where titel = '" + titel + "'));" +
                             "INSERT INTO Regisseurs (voornaam, achternaam, FilmID) VALUES('" + rVoornaam + "' , '" + rAchternaam + "', (SELECT FilmID from Films where titel = '" + titel + "'));";
        command = new SqlCommand(queryString, con);

Can someone please help me with this? I can't figure it out.

Based from the error message, it says that "There are fewer columns in the INSERT statement". So I'm guessing that there are still columns in your table that you need to include in your insert statement — prtdomingo, Jan 24 '17 at 09:57
[Beware of SQL injection attack, your code is wide open to it.](http://stackoverflow.com/q/14376473/335858) — Sergey Kalinichenko, Jan 24 '17 at 09:59
You have more values in "VALUE (...." that fields in INSERT side. — raBinn, Jan 24 '17 at 09:59
`VALUES('" + titel + "', " + beoordeling + ")` is missing a ' at the end. — Takarii, Jan 24 '17 at 10:00
Add a breakpoint, look at `queryString`, copy it value, paste the result in your management studio and execute it. — McNets, Jan 24 '17 at 10:03
Takarii, if I use the ' then I declare it as an string right? beoordeling is a decimal for me. — Tim Hellegers, Jan 24 '17 at 10:04

score 3 · Accepted Answer · edited Sep 22 '17 at 02:19

Use parametererized queries and do not use string concatination. This is to prevent sql injection attacks but also errors with the values like forgetting to make sure strins are escaped (if a string contains a ' for example).
If you have multiple queries each unique parameter value should have its own parameter name/value
Wrap your ado.net database types (SqlConnection, SqlCommand, etc) in using blocks if they are disposable
Never reuse connections as global objects, create, use, and destroy them when needed.

Here is the updated code with 1 statement, you can append additional statements to this and add more parameters as necessary.

var query = "INSERT INTO Acteurs (voornaam, achternaam, FilmID) SELECT @a1Voornaam, @a1Achternaam, FilmID from Films WHERE titel = @title";

using(var con = new SqlConnection("connection string here"))
using(var command = new SqlCommand(queryString, con))
{
  command.Parameters.Add(new SqlParameter("@a1Voornaam", SqlDbType.VarChar){Value = a1Voornaam});
  command.Parameters.Add(new SqlParameter("@achternaam", SqlDbType.VarChar){Value = achternaam});
  command.Parameters.Add(new SqlParameter("@title", SqlDbType.VarChar){Value = title});

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

@TimHellegers - be sure to use the proper parameter types too (`SqlDbType` enumeration). As an example, if you are using nvarchar instead of varchar in the database then specify that instead. — Igor, Jan 24 '17 at 10:10
Thanks, I tried it with your code above. It worked! Now I'll try to apply this for the other query's — Tim Hellegers, Jan 24 '17 at 10:16
This is good, but there's still room for improvement -- if you don't specify the parameter size explicitly, it will be inferred from the value length. If that length differs per query (as you'd expect) it leads to [plan cache pollution](https://blogs.msdn.microsoft.com/ivandonev/plan-cache-pollution-or-how-important-it-is-to-properly-define-parameters-in-code/). To avoid this, specify the size explicitly. — Jeroen Mostert, Jan 24 '17 at 11:20

score 0 · Answer 2 · answered Jan 24 '17 at 10:05

Perhaps one of your values is ');

That would terminate the INSERT statement early, and cause the error.

                                                   |
                                                   V
  INSERT INTO Films (titel, beoordeling) VALUES('');,'anything');

You should use SqlParameters instead of string concatenation.

score 0 · Answer 3 · answered Feb 02 '17 at 21:19

Are you using TextBoxes? I can't tell for sure. Try something like this, and change to suit your specific needs.

using System.Data.SqlClient;

protected void Button1_Click(object sender, EventArgs e)
{
SqlConnection con = new SqlConnection(System.Configuration.
ConfigurationManager.ConnectionStrings["con"].ToString());
    try
    {
        string query = "insert into UserDetail(Name,Address) 
        values('" + txtName.Text + "','" + txtAddress.Text + "');";
        SqlDataAdapter da = new SqlDataAdapter(query, con);
        con.Open();
        da.SelectCommand.ExecuteNonQuery();
        con.Close();
        lblmessage.Text = "Data saved successfully.";
    }
    catch
    {
        con.Close();
        lblmessage.Text = "Error while saving data.";
    }

}

Why is my SQL code in C# not working?

3 Answers3