Editing data into SQL with C#

Question

I have this method for Editing data but I don't know how to write the code... Until now I have this which I don't really understand and I have an error in it. It says incorrect syntax near '('.

        public void EditMember(Member member)
        {
            string Name = member.Name;
            string Surname = member.Surname;
            string EntryDate = member.EntryDate.ToString("dd.MM.yyyy");
            string Status = member.Status;

            sqlConnection.Open();
            sqlCommand = new SqlCommand(
                "UPDATE Members SET (Name, Surname, EntryDate) VALUES('" + Name + "','" + Surname + "','" + EntryDate + "')' WHERE'(' Id '='" + member.Id + "')",
                sqlConnection);
            sqlCommand.ExecuteNonQuery();
            sqlConnection.Close();
        }

The problem is when I start to write WHERE

Help please.

SQL Injection alert - you should not concatenate together your SQL statements - use parametrized queries instead to avoid SQL injection — marc_s
– marc_s, Commented Jun 5, 2013 at 11:42
always ALWAYS use parameterized queries and DON'T use dynamic SQL. Read up on it here. — Wim Ombelets
– Wim Ombelets, Commented Jun 5, 2013 at 11:42

Jon Skeet · Accepted Answer · 2013-06-05 11:49:07Z

3

Please read all of this answer, not just the first part

There are multiple issues here. The most immediate problem is here:

"')' WHERE'('

That's acting as if you're trying to quote the bracket. That "should" be:

"') WHERE ('

At that point it would look like a valid (but bad) INSERT command... but your use of VALUES which doesn't look like it's a valid way of updating in T-SQL anyway.

However, you shouldn't use this approach at all. It's error-prone, hard to read, and most importantly prone to SQL injection attacks.

Instead, you should use parameterized SQL:

string sql = @"UPDATE Members
               SET Name = @Name, Surname = @Surname, EntryDate = @EntryDate
               WHERE Id = @Id";

using (var connection = new SqlConnection(...))
{
    connection.Open();
    using (var command = new SqlCommand(sql, connection))
    {
        command.Parameters.Add("@Name", SqlDbType.NVarChar).Value = member.Name;
        command.Parameters.Add("@Surname", SqlDbType.NVarChar).Value = member.Surname;
        command.Parameters.Add("@EntryDate", SqlDbType.DateTime).Value = member.EntryDate;
        command.Parameters.Add("@Id", SqlDbType.NVarChar).Value = member.Id;
        int rows = command.ExecuteNonQuery();
        // TODO: Work out what to do if rows isn't 1
    }
}

(With adjustments for the appropriate data types, of course.)

edited Jun 5, 2013 at 11:49

answered Jun 5, 2013 at 11:43

Jon Skeet

1.5m893 gold badges9.3k silver badges9.3k bronze badges

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

2 Comments

Jon Skeet Over a year ago

@marc_s: Yup, was editing for that. (I'd appreciate it if you'd check the update though :)

marc_s Over a year ago

Looks much better now, indeed ! :-)

marc_s · Accepted Answer · 2013-06-05 11:44:59Z

You should NEVER EVER concatenate together your SQL statements with user input.

Instead : use parametrized queries - they're easy to use, avoid SQL injection, and improve performance.

Try code something like this:

string updateStmt = "UPDATE dbo.Members SET Name = @Name, Surname = @Surname, EntryDate = @EntryDate WHERE Id = @ID";

sqlCommand = new SqlCommand(updateStmt, sqlConnection);

sqlCommand.Parameters.Add("@Name", SqlDbType.VarChar, 100).Value = name;
sqlCommand.Parameters.Add("@Surname", SqlDbType.VarChar, 100).Value = surname;
sqlCommand.Parameters.Add("@EntryDate", SqlDbType.DateTime).Value = entrydate;
sqlCommand.Parameters.Add("@ID", SqlDbType.Int).Value = member.Id;

sqlConnection.Open();
sqlCommand.ExecuteNonQuery();
sqlConnection.Close();

Steve · Accepted Answer · 2013-06-05 11:48:21Z

The correct syntax for an update statement is

  "UPDATE Members SET Name = @name, Surname = @surname, EntryDate = @date WHERE id=@id"

Said that, you should use parameterized query like this

    public void EditMember(Member member)
    {
        string Name = member.Name;
        string Surname = member.Surname;
        string EntryDate = member.EntryDate.ToString("dd.MM.yyyy");
        string Status = member.Status;

        sqlConnection.Open();
        sqlCommand = new SqlCommand("UPDATE Members SET Name = @name, Surname = @surname, " + 
                         "EntryDate = @date " + 
                         "WHERE Id = @id", sqlConnection);
        sqlCommand.Parameters.AddWithValue("@name", Name);
        sqlCommand.Parameters.AddWithValue("@surname", Surname);
        sqlCommand.Parameters.AddWithValue("@date", EntryDate);
        sqlCommand.Parameters.AddWithValue("@id", Status);
        sqlCommand.ExecuteNonQuery();
        sqlConnection.Close();

As a side note, keep in mind that AddWithValue is a simple way to add parameters to you query, but if the perfomance of this query is critical it is better to use a fully defined parameter with the datatype that matches exactly your database column's type and with the exact size.

Wim Ombelets · Accepted Answer · 2013-06-05 11:44:53Z

0

Remove the quotes from around the WHERE and you should be fine. Please heed the warnings given in the comments about SQL injection attacks.

answered Jun 5, 2013 at 11:44

Wim Ombelets

5,2853 gold badges43 silver badges59 bronze badges

Comments

Olivier Jacot-Descombes · Accepted Answer · 2013-06-05 11:59:07Z

Your code has syntax error for update and also SQLInjection issue.

You need to pass parameters to update query rather than passing direct values.

It should be as follows:

public void EditMember(Member member)
{
    string Name = member.Name;
    string Surname = member.Surname;
    string EntryDate = member.EntryDate.ToString("dd.MM.yyyy");
    string Status = member.Status;

    sqlConnection.Open();
    sqlCommand = new SqlCommand("UPDATE Members SET Name=@Name, Surname=@Sirname, EntryDate=@EntryDate WHERE Id = @id", sqlConnection);
    sqlCommand.parameters.AddparameterWithValue("@Name",Name);
    sqlCommand.parameters.AddparameterWithValue("@Surname",Surname);
    sqlCommand.parameters.AddparameterWithValue("@EntryDate",EntryDate);
    sqlCommand.parameters.AddparameterWithValue("@Id",Id);
    sqlCommand.ExecuteNonQuery();
    sqlConnection.Close();
}

Sumit Gupta · Accepted Answer · 2013-06-05 12:10:56Z

-1

Edit the post to make correct answer:

i.e. you don't need brackets in where clause. And yes the better query is

  "UPDATE Members SET Name=@Name, Surname=@Surname, EntryDate=@EntryDate WHERE  Id=@ID"

and then you add @Name, @Surname, .. etc through parameter of command object.

edited Jun 5, 2013 at 12:10

answered Jun 5, 2013 at 11:45

Sumit Gupta

2,2024 gold badges31 silver badges47 bronze badges

3 Comments

marc_s Over a year ago

This syntax UPDATE Members SET (Name, Surname, EntryDate) VALUES(@Name,@Surname,@EntryDate) is not valid in SQL Server ...

Olivier Jacot-Descombes Over a year ago

This mixes the syntaxes of INSERT and UPDATE statements!

Sumit Gupta Over a year ago

I am sorry about syntax, I use MYSql and SqlServer a like and forgot the database in question is SQL Server, the above syntax is however correct in MySql Database server. Thank for correction.

Collectives™ on Stack Overflow

Editing data into SQL with C#

6 Answers 6

2 Comments

Comments

Comments

Comments

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

2 Comments

Comments

Comments

Comments

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related