ExecuteNonQuery() SQLException Incorrect Syntax Near ',' WPF

Question

I am trying to run a SQL command that saves all the items in a column back to a database table. The users enter in this information, and when they close the window the newly entered information should get saved back to a database. I am attempting to do this using a SQL connection and SQL command. The issue is when I enter in the information and close the window, I get an error at the comm.ExecuteNonQuery() that says 'Incorrect syntax near ','.' I am unsure exactly where in my command there is an issue considering I entered my syntax in a online SQL syntax checker and everything came out fine. Here is my code for the Window_Closing event below:

public void Window_Closing(object sender, CancelEventArgs e)
    {
        var checkNoValue = ((DataProperties)gridView.SelectedItem).Check_No;
        var checkDateValue = ((DataProperties)gridView.SelectedItem).Check_Date;

        SqlConnection conn;
        SqlCommand comm;
        string connstring = "server = testserver; database = test; user = someusername; password = somepassword";
        conn = new SqlConnection(connstring);
        conn.Open();
        comm = new SqlCommand("insert into DeductionInfo (" + gridView.Columns["checkNo"] + ", " + gridView.Columns["checkDate"] + ") values (" + checkNoValue + ", " + checkDateValue + ")", conn);

        comm.ExecuteNonQuery();
        MessageBox.Show("Saved");
    }

I am trying to insert the new data into the DeductionInfo datatable located in my database. I think I can sort of guess what the issue is, since I have checkNoValue and checkDateValue set to my gridView.SelectedItem property, however I am unsure how to set that to be each value in those specific columns (I am using Telerik's RadGridView by the way). Keep in mind that Check_No is an integer and Check_Date is a datetime. Am I tackling this all wrong or am I on the right track? This is the first time I've handled SQL queries in WPF and C#, so I'm pretty new to this. Any help would be appreciated :)

Try using parameters instead of creating the insert command with variables. — vintastic, Aug 22 '17 at 15:43
What are the actual names of your columns in the DeductionInfo table? These aren't dynamic... — mm8, Aug 22 '17 at 15:46
You should read about SQL injection : https://stackoverflow.com/questions/7505808/why-do-we-always-prefer-using-parameters-in-sql-statements — TinkeringMatt, Aug 22 '17 at 15:46
Thanks for the responses. The actual names of my columns are "Check_No", and "Check_Date" and these are defined in a separate class called "DataProperties", hense my cast for checkNoValue and checkDateValue. — GamerTalks, Aug 22 '17 at 15:52
I meant the actual names of columns in the table in the database, not your properties... — mm8, Aug 22 '17 at 15:55
My apologies. In the table it's called checkNo and checkDate, but I definitely understand the confusion. — GamerTalks, Aug 22 '17 at 15:56
Then it should be insert into DeductionInfo (checkNo, checkDate) ... — mm8, Aug 22 '17 at 16:03

mm8 · Accepted Answer · 2017-08-22T16:03:25.063

2

You should always use parameters when executing dynamic SQL queries and you should always dispose IDisposables as soon as you are done using them.

Besides, the column names of a table in a database aren't dynamic. You need to know the actual names of these to be able to insert some data into them.

Try this:

public void Window_Closing(object sender, CancelEventArgs e)
{
    var checkNoValue = ((DataProperties)gridView.SelectedItem).Check_No;
    var checkDateValue = ((DataProperties)gridView.SelectedItem).Check_Date;

    SqlConnection conn;
    SqlCommand comm;
    string connstring = "Server = testserver; Database = test; User Id = test; Password = somepassword;"
            conn = new SqlConnection(connstring);
    conn.Open();
    comm = new SqlCommand("insert into DeductionInfo (checkNo, checkDate) values (@checkNoValue, @checkDateValue)", conn);
    comm.Parameters.AddWithValue("@checkNoValue", checkNoValue);
    comm.Parameters.AddWithValue("@checkDateValue", checkDateValue);

    comm.ExecuteNonQuery();
    comm.Dispose();
    conn.Dispose();

    MessageBox.Show("Saved");
}

edited Aug 22 '17 at 16:03

answered Aug 22 '17 at 15:53

mm8

163,881
10
57
88

Thank you. You gave a very clear answer to this question. Like I said, when dealing with databases and SQL Queries, I'm pretty new to it. Even with all the research I did prior to posting this question I was not exposed to the idea of parameters. It definitely makes a lot of sense. I will let you know if it works. – GamerTalks Aug 22 '17 at 15:55
That got rid of my initial error. Now it says 'The parameterized query '(@checkNoValue nvarchar(4), @checkDateValue nvarchar(4000)) insert' expects the parameter '@checkDateValue', which was not supplied'. I am not entirely sure what it is saying here... – GamerTalks Aug 22 '17 at 16:12
Make sure that your checkDateValue variable contains a value. – mm8 Aug 22 '17 at 16:14
https://stackoverflow.com/questions/23448403/the-parameterized-query-expects-the-parameter-units-which-was-not-supp – mm8 Aug 22 '17 at 16:30
It does, although it's a DateTime value so I'm not sure if that'd affect anything. The property in DataProperties is DateTime and the column in the datatable in the database is datetime as well. – GamerTalks Aug 22 '17 at 16:31
According to the error message it's a nvarchar(4000). – mm8 Aug 22 '17 at 16:37
Gotcha. so in my DataProperties, checkDate is a DateTime? because that is what it's set as in my database table. I am unsure if that affects much however. After looking at the link you posted above, using if/else statements to check for nulls did help, but another error is thrown: "into column 'claimid', table 'test.DeductionInfo'; column does not allow nulls. INSERT fails." This error doesn't make any sense, because I don't make any calls to my 'claimid' column which is completely separate... – GamerTalks Aug 22 '17 at 16:43
Well, you need to if the DeductionInfo contains such a column. You cannot insert a new row without supplying a claimid then. – mm8 Aug 22 '17 at 16:49
Gotcha. I guess I just got confused because Check_No and Check_Date are the only null columns, and every other column in the database will be automatically generated at a later time. I apologize for not completely getting this, but I thank you for at least being patient haha – GamerTalks Aug 22 '17 at 16:58

score 1 · Answer 2 · answered Aug 22 '17 at 15:48

1

Here's a proper way to dispose of your connection when it's over, and how to pass parameter to a SQL query. This will save you errors like the one you're having right now and also protect you against SQL Injection

string sql = "SELECT empSalary from employee where salary = @salary";

using (SqlConnection connection = new SqlConnection(/* connection info */))
using (SqlCommand command = new SqlCommand(sql, connection))
{
    var salaryParam = new SqlParameter("salary", SqlDbType.Money);
    salaryParam.Value = txtMoney.Text;

    command.Parameters.Add(salaryParam);
    var results = command.ExecuteReader();
}

source : Why do we always prefer using parameters in SQL statements?

answered Aug 22 '17 at 15:48

TinkeringMatt

181
1
9

Thanks for the response, I will try it out. I did a lot of research into this issue but I was never presented with the idea to use parameters. It helps a lot. – GamerTalks Aug 22 '17 at 15:53
Also make sure you add the parameters in your c# code in the same order as they appear in your query. this caused me many headaches in the past – TinkeringMatt Aug 22 '17 at 15:55

ExecuteNonQuery() SQLException Incorrect Syntax Near ',' WPF

2 Answers2