0

I'm trying to insert data into my database via SqlCommand.

It's very simple but I can't understand the exception message:

System.Data.SqlClient.SqlException: 'Incorrect syntax near the keyword 'AS'

My code:

SqlConnection cn = new SqlConnection("**CONNECTION STRING IS WORKING FINE, CHECKED**");

string tsql = "INSERT INTO CAMADA VALUES (" + id + ", " + team + "," + shift + "," + starttime + "," + jobdate + "," + PM;

SqlCommand cmd = new SqlCommand(tsql,cn);
cmd.CommandType = CommandType.Text;

cn.Open();
cmd.ExecuteNonQuery();

Any help will be very appreciated.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • 7
    Your code is vulnerable to [SQL Injection](https://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work) attack. You should always use parameters in your query to be protected. – dotnetom Oct 05 '18 at 03:39
  • Any triggers on that table? – John Wu Oct 05 '18 at 03:39
  • 3
    You're missing closing parenthesis after `PM`. Also avoid SQL injection by using parameterized query. – Tetsuya Yamamoto Oct 05 '18 at 03:39
  • 2
    Look at exception message. Then on exception object. There is an info, that your query string having an issue. If you look at query string, which may looks like `INSERT INTO CAMADA VALUES ("42", "red_team","5","01-01-2018","10:00","true` - you will see, that you forget to close the brackets. – vasily.sib Oct 05 '18 at 03:40
  • 1
    You also missed enclosing the string and datetime literals in single quotes. A parameterized query will avoid the need to do that. – Dan Guzman Oct 05 '18 at 04:24

1 Answers1

1

Your code have three major issues -

  1. It's vulnerable to SQL Injection attacks.
    Never concatenate strings with user inputs into your sql. Use parameters instead.
    Learn how parameters protect you from SQL Injection.

  2. It's not disposing the instance of SqlConnection as it should, so it can be added or returned to the connection pool.

  3. You did not specify the columns list in your insert statement.
    This means that if you ever add a column to your table, or simply reorder the columns, your insert statement will fail with an error.

The code below contain an example of the proper way to execute SQL statements.
Be sure to read and understand the comments, they are there to help you.
Please note, however, that I had to guess the data types of the parameters - you might need to change them.
Also, be sure to set the size of the parameter when sending strings (char/nchar/varchar/nvarchar) or binary data (binary/varbinary) to the database.

// proper spacing and indentations makes the code easier to read thus more maintainable.
// Also, Always specify the columns list in an insert statement.
var tsql = @"INSERT INTO CAMADA (id, team, shift, starttime, jobdate, pm) 
             VALUES (@id, @team, @shift, @starttime, @jobdate, @pm)";
// use the using statement to ensure the SqlConnection is closed and disposed when done.
// This is important not only because it's an IDispsable, but also because that 
// if you don't dispose it, the connection will not return to the connection pool
using(var cn = new SqlConnection("**CONNECTION STRING IS WORKING FINE, CHECKED**"))
{
    // SqlCommand is also an IDisposable
    using(var cmd = new SqlCommand(tsql,cn))
    {
        // CommandType.Text is the default, no need to set it explicitly

        // Use parameters to avoid all kinds of nasty stuff like SQL injection,
        // but also having to handle DateTime string literals formats 
        cmd.Parameters.Add("@id", SqlDbType.Int).Value = id;
        cmd.Parameters.Add("@team", SqlDbType.Int).Value = team;
        cmd.Parameters.Add("@shift", SqlDbType.Int).Value = shift;
        cmd.Parameters.Add("@starttime", SqlDbType.DateTime).Value = starttime;
        cmd.Parameters.Add("@jobdate", SqlDbType.DateTime).Value = jobdate;
        // Of course, the size is also a guess...
        cmd.Parameters.Add("@pm", SqlDbType.VarChar, 2).Value = pm;
        cn.Open();
        cmd.ExecuteNonQuery();    
    }
}
Zohar Peled
  • 79,642
  • 10
  • 69
  • 121