-2

I'm trying to add information to a table when it is input, but get an error saying there is incorrect syntax, and I have no clue where I've gone wrong.. I'm using C# and SQL Server.

Here is my code:

Con.Open();

string query = "insert into tablepassengers values(" + tbpassno.Text + "','" + tbname.Text + "','" + tbaddress.Text + "','" + tbnumber.Text + "','" + cbnationality.SelectedItem.ToString() + "')'";

SqlCommand cmd = new SqlCommand(query, Con);
cmd.ExecuteNonQuery();

MessageBox.Show("recorded");
Con.Close();
Charlieface
  • 52,284
  • 6
  • 19
  • 43
yana
  • 11
  • 5
  • ive been watching a video and this is what they did too + it worked for them – yana May 08 '22 at 18:48
  • 2
    `(" + tbpassno.Text + "','"` missing `'` at the beginning. Should be `('" + tbpassno.Text + "','"`? – Matt May 08 '22 at 18:48
  • 9
    Ideally, you should use parameterization [docs](https://learn.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlcommand.parameters?view=dotnet-plat-ext-6.0) – Matt May 08 '22 at 18:51
  • @Matt ive added in the `'` btu now it says [unclosed quotation mark after the character string ".] -- im also on a deadline and was told to use this method + am not sure about parameterisation docs.. – yana May 08 '22 at 18:55
  • 6
    Can you see what would happen with the apostrophes if someone had a name with an apostrophe? The best way to pass parameters to an SQL query is with SQL parameters: [Why do we always prefer using parameters in SQL statements?](https://stackoverflow.com/q/7505808/1115360) – Andrew Morton May 08 '22 at 18:55
  • Please report the *exact* error text when asking a question. It's unlikely that it says, "Incorrect error". "Incorrect syntax" is more likely. But the exact text of an error message often gives you the best clue as to what is going on. – Wyck May 08 '22 at 19:00
  • Does this answer your question? [Why do we always prefer using parameters in SQL statements?](https://stackoverflow.com/questions/7505808/why-do-we-always-prefer-using-parameters-in-sql-statements) – Charlieface May 08 '22 at 19:41
  • 1
    Also you should specify column names that you are inserting into. `INSERT INTO table (Col1, Col2...) VALUES ...` And you need to dispose your connection and command objects with `using` – Charlieface May 08 '22 at 19:42
  • 3
    27th question I've read today that uses awful, inejction-hack prone SQL. Where are you getting these tutorials that tell you to do this? *this is what they did too* - link us to it so we can give them a roasting too; we'll never stem the tide of crap by trying to educate the students after they've been taught wrong; need to try and educate the teachers – Caius Jard May 08 '22 at 19:58
  • @CaiusJard i got told in school to do it like this too, the video im trying to follow along right now is https://youtu.be/q8OjCp_mpJA ,, one of the teachers told us to try follow along these kind of videos and try to understand whats going on :// – yana May 08 '22 at 20:37
  • 1
    Writing code like that caused the compromise and theft of data, including pictures, names and home addresses of hundreds of thousands of children that use VTech products: https://www.bbc.co.uk/news/technology-34944140 - you probably don't want something like that on your conscience; we warn developers that do it, then fire them if they carry on. I'd raise a complaint about the quality of teaching at your institution, especially if you're paying big money for it; my institution was emphatic about parameterizing right from the start – Caius Jard May 08 '22 at 20:50

1 Answers1

3

Change the query to:

string query = "insert into tablepassengers values('" + tbpassno.Text + "','" + tbname.Text + "','" + tbaddress.Text + "','" + tbnumber.Text + "','" + cbnationality.SelectedItem.ToString() + "')";

Basically, you forgot a ' after opening the parenthesis and you added an extra ' after closing it.

Though, to be honest, it'd be better to use parameters for security purposes.

For example,

string query = "insert into tablepassengers values(@1, @2, @3, @4, @5)";
SqlCommand cmd = new SqlCommand(query, Con);

cmd.Parameters.AddWithValue("@1", tbpassno.Text);
cmd.Parameters.AddWithValue("@2", tbname.Text);
cmd.Parameters.AddWithValue("@3", tbaddress.Text);
cmd.Parameters.AddWithValue("@4", tbnumber.Text);
cmd.Parameters.AddWithValue("@5", cbnationality.SelectedItem.ToString());

cmd.ExecuteNonQuery();

For more, read SQLCommand.Parameters.

  • @CaiusJard I edited my answer to add the finished solution. – Stratis Dermanoutsos May 09 '22 at 06:30
  • 2
    It's also worth mentioning https://www.dbdelta.com/addwithvalue-is-evil/ - AddWithValue does cause some performance problems in SQLServer (but not necessarily other databases) that can be avoided by creating the parameters in another way - but it's infinitely preferable over sql injection! – Caius Jard May 09 '22 at 07:13