What's wrong with this SQL Insert statement of mine?

Question

I am trying to add a record to my table, however I'm getting an exception after it attempts to do so. I am able to open the connection successfully, (my first messagebox shows up) but after that I get the exception. Here's my code:

Private Sub btnUpdateInfo_Click(sender As Object, e As EventArgs) Handles btnUpdateInfo.Click
    Dim con As New SqlConnection
    Dim cmd As New SqlCommand
    con = New SqlConnection("Data Source=localhost\SQLEXPRESS;Initial Catalog=CISDB;Integrated Security=SSPI;")
    Try
        cmd.CommandText = "INSERT INTO customers (FirstName,LastName) VALUES (txtFirstName.Text, txtLastName.Text)"
        cmd.Connection = con
        con.Open()
        MsgBox("Connection Open ! ")
        cmd.ExecuteNonQuery()
        MsgBox("Record inserted")
        con.Close()
    Catch ex As Exception
        MsgBox("Error!")
    End Try
End Sub

Its doubtful you want to insert the literal "txtFirstName.Text" for the first name. Use SQL Parameters. Remove the Try/Catch - it is hiding important information from you — Ňɏssa Pøngjǣrdenlarp, Nov 30 '16 at 19:42
Fortunately this failed for you. It failed because you are trying to insert string literals instead of the values. However, you need to use parameters here and not add textbox values like this as it is vulnerable to sql injection. And as already mentioned that catch is a horrible design. It is barely better than an empty catch. You know that an error happened but since you don't log or present any useful information you have no way of knowing what happened. — Sean Lange, Nov 30 '16 at 19:44
@SeanLange actually it fails because SQL has no knowledge of the symbols `txtFirstName.Text` and `txtLastName.Text`. They are not string literals in the query, so it would throw either an unknown symbol error or a syntax error if the `.` throws it off. — D Stanley, Nov 30 '16 at 19:56
@DStanley - Actually it is a string literal that when passed to the sql engine will fail. We are saying the same thing here. And fortunately for the OP it failed before bobby tables paid them a visit. :D — Sean Lange, Nov 30 '16 at 20:26
@SeanLange Wouldn't it have to be in quotes to be considered a string literal? — D Stanley, Nov 30 '16 at 20:34
@Sam Lev Please don't forget to mark the answer as accepted that helped you the most - THanks — glant, Nov 30 '16 at 20:51
@DStanley once it hits sql yes, but wouldn't it be a string literal in C#? :) — Sean Lange, Nov 30 '16 at 20:51
@SeanLange I am distinguishing `"INSERT INTO customers (FirstName,LastName) VALUES (txtFirstName.Text, txtLastName.Text)"` and `"INSERT INTO customers (FirstName,LastName) VALUES ('txtFirstName.Text', 'txtLastName.Text')"` The first would be an error, the second would insert wrong data. It's somewhat moot as both are wrong, though. — D Stanley, Nov 30 '16 at 21:14
@DStanley 100% agreement. We have been both been saying the same thing all along. :D — Sean Lange, Nov 30 '16 at 21:30

Fabio · Answer 1 · 2016-12-01T17:11:32.663

For future readers - Sql parameters will save a lot of your and your coworkers time.

Private Sub btnUpdateInfo_Click(sender As Object, e As EventArgs) Handles btnUpdateInfo.Click
    Dim connString = "Data Source=localhost\SQLEXPRESS;Initial Catalog=CISDB;Integrated Security=SSPI;"
    Using connection As New SqlConnection(connString)
        Using command As New SqlCommand(connection)
            command.CommandText = "INSERT INTO customers (FirstName,LastName) VALUES (@FirstName, @Lastname)"
            Dim params As SqlParameter() =
            {
                New SqlParameter With { .ParameterName = "@FirstName", .SqlDbType.VarChar, .Value = txtFirstName.Text },
                New SqlParameter With { .ParameterName = "@LastName", .SqlDbType.VarChar, .Value = txtLastName.Text },
            }
            command.Parameters.AddRange(params)
            connection.Open() 
            command.ExecuteNonQuery()
            ' Inserted
        End Using
    End Using
End Sub

Same for try.. catch(as @Plutonix has noticed in the comments) - if you will get "real" exception you will find reason and solution much faster

OP Here- thanks for this! I got rid of the try/catch and I was then able to see the actual error message and diagnose my issue. I now will begin to learn about parameters as many have suggested- this is a great example. Now that it's all working, would it be good practice to put the try/catch back in? — Sam Lev, Dec 02 '16 at 15:25

glant · Accepted Answer · 2016-11-30T20:25:56.247

0

You need to look at the exception message (ex.Message) see what the issue is...If you have an error similar to multipart identifier then try this query string instead of your current query string for a quick test.

    cmd.CommandText = "INSERT INTO customers (FirstName,LastName) VALUES ('" & txtFirstName.Text & "', '" & txtLastName.Text & "')"

Check out parameterized query as previously indicated

edited Nov 30 '16 at 20:25

answered Nov 30 '16 at 20:18

glant

479
6
19

What's wrong with this SQL Insert statement of mine?

2 Answers2