Missing operator in expression?

Question

So obviously it says I'm missing an operator in my query but I can't find where?

Dim updateStatement As String =
        "UPDATE Customers SET " &
        "Name = """ & txtName.Text & ", """ &
        "Address = """ & txtAddress.Text & ", """ &
        "City = """ & txtCity.Text & ", """ &
        "State = """ & txtState.Text & ", """ &
        "ZipCode = """ & txtZipCode.Text & ", """

There error looks like this:

A first chance exception of type 'System.Data.OleDb.OleDbException' occurred in
System.Data.dll

Additional information: 
Syntax error (missing operator) in query expression 
'"Johnson, Ajith, "Address = "2200 Old Germantown Road, "City = "McGregor, 
"State = "CA, "ZipCode = "55555          , "'.

Note that this is happening when I try to update a record

Edit: Also I am not using AddWithValue for a reason.

Edit 2: Yes I'm aware that this is a bad way to do this but this is how the instructor wants it.

Please please please look into paramaterized sql statements, you are extremely vunerable to SQL injection attacks with this code. — asawyer, Mar 28 '14 at 15:48
I agree with @asawyer that this is very bad practice - however, you should be able to get an idea of where your problem is by writing out the value of your updateStatement variable to the console before you try and execute it. Also - you are missing a WHERE clause? — Julian Joseph, Mar 28 '14 at 15:52
As far as the question is concerned, It looks like you need commas after your closing quotes on each property except the zipcode obviously... — Mike_OBrien, Mar 28 '14 at 15:53

Patrick Hofman · Accepted Answer · 2014-03-28T16:00:10.760

3

The solution is this.

Dim updateStatement As String =
    "UPDATE Customers SET " &
    "Name = """ & txtName.Text & """, " &
    "Address = """ & txtAddress.Text & """, " &
    "City = """ & txtCity.Text & """, " &
    "State = """ & txtState.Text & """, " &
    "ZipCode = """ & txtZipCode.Text & """"

But as other pointed out: NEVER EVER EVER USE THIS!

It is unreadable, unmaintainable, and a hazard for SQL injection.

Use this instead:

Dim updateStatement As String =
        "UPDATE Customers SET " &
        "Name = @name, Address = @address, City = @city, State = @state, ZipCode = @zipcode"

And fill those parameters in your OleDbCommand. If you are using Access or alike, you should use ? instead of named parameters.

In a production environment, you also like the benefit of using parameters: you will gain the possibility from the database to cache the execution plan, etc. since the hash of the statements keeps the same. So if you run this 1000 times, the database doesn't need to 'rethink' how to execute this: performance gain!

edited Mar 28 '14 at 16:00

answered Mar 28 '14 at 15:51

Patrick Hofman

153,850
22
249
325

Make your answer better by explaining why. It's not obvious – rory.ap Mar 28 '14 at 15:52
2

Doesn't the last comma still break it? There's nothing after that comma. – rory.ap Mar 28 '14 at 15:53
If you are required to write it that way, then you could use `System.Web.HttpUtility.HtmlEncode(txtName.Text)` , and it it should mess up potential injections. It's along the lines of something I used to have to do in PHP. – Jimmy Smith Mar 28 '14 at 16:02
1

@JimmySmith: You are right, but you will also loose the possibility from the database to cache the execution plan, etc since the hash changes every time you create a statement. – Patrick Hofman Mar 28 '14 at 16:03
Patrick is totally correct. Without parameterized queries, it has to create a new query plan each time on SQL Server. That is why I've used them in code that isn't subject to SQL injection, but I just want to run faster. – Jimmy Smith Mar 28 '14 at 16:08

score 0 · Answer 2 · edited May 23 '17 at 10:26

I will never teach you to use a string concatenation to build an sql statement. If someone tries to convince you that it is not important then you should point him/her to some page about Sql Injection

By the way, your error is probably caused by the comma at the end of the ZipCode value.

And keep in mind that an UPDATE without a WHERE statement updates all the records in the table

Instead you should code the UPDATE using a parameterized query

    Dim updateStatement As String =
    "UPDATE Customers SET Name=?, " & _
    "Address = ?, City = ?, State = ?, ZipCode = ? " & _
    "WHERE YourPrimaryKeyField = ?"

    OleDbCommand cmd = new OleDbCommand(updateStatement, connection)
    cmd.Parameter.AddWithValue("@p1", txtName.Text)
    cmd.Parameter.AddWithValue("@p2", txtAddress.Text)
    cmd.Parameter.AddWithValue("@p3", txtCity.Text )
    cmd.Parameter.AddWithValue("@p4", txtState.Text )
    cmd.Parameter.AddWithValue("@p5", txtZipCode.Text )
    cmd.Parameter.AddWithValue("@p6", txtPrimaryKeyValue.Text)
    cmd.ExecuteNonQuery()

Now, a part from the Sql Injection problem, look at the query text. Do you think that this is a lot more understandable than the messy concatenation above?

Missing operator in expression?

2 Answers2