Invalid column name? c# sql

Question

Why isnt this working? it says invalid column name when i try to remove something after i added it

    private void btnRemoveCommand_Click(object sender, EventArgs e)
    {
        connection = new SqlConnection(connectionString);
        connection.Open();         

        for (int i = 0; i < listBox1.SelectedItems.Count; i++)
        {
            var sql = "DELETE FROM Commands WHERE commandName = " + listBox1.SelectedItems[i] + "";

            listBox1.Items.Remove(listBox1.SelectedItems[i]);               
            SqlCommand cmd = new SqlCommand(sql, connection);
            cmd.ExecuteNonQuery();
        }
        connection.Close();
    }

this is the event that handles the addCommand to the database

private void btnAddCommand_Click(object sender, EventArgs e)
{
   var sql = "INSERT INTO Commands(commandName, pathToCommand) VALUES(@commandName, @pathToCommand)";
                using (var connection = new SqlConnection(connectionString))
                {
                    connection.Open();

                    SqlCommand cmd = new SqlCommand(sql, connection);

                    cmd.Parameters.AddWithValue("@commandName", tbxCommand.Text);
                    cmd.Parameters.AddWithValue("@pathToCommand", tbxPathToCommand.Text);

                    int affectedRows = cmd.ExecuteNonQuery();
                }
}

Ray Krungkaew · Accepted Answer · 2018-01-30 09:25:05Z

1

Change

var sql = "DELETE FROM Commands WHERE commandName = " + listBox1.SelectedItems[i] + "";

to

var sql = "DELETE FROM Commands WHERE commandName = '" + listBox1.SelectedItems[i] + "'";

answered Jan 30, 2018 at 9:25

Ray Krungkaew

7,0071 gold badge19 silver badges29 bronze badges

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

3 Comments

wille480 Over a year ago

Wow! thanks for the help! really appriciated : ), worked flawlessly

wille480 Over a year ago

I'll give you the answer on this post whenever i can

Peter B Over a year ago

Even better (and then I mean MUCH better) would be to use SqlParameter, that way you never have to worry about (a) enclosing quotes, (b) embedded quotes that need escaping, (c) SQL injection. E.g. see csharp-station.com/Tutorial/AdoDotNet/Lesson06. You even used in the second code block, should also use it in the first!

GarethD · Accepted Answer · 2018-01-30 09:48:00Z

1

First thing is first, always always, always use parameterised queries. No exceptions. Ever.

Next, use using blocks for objects that implement iDisposable, to ensure your unmanaged resources are properly cleaned up.

Finally, when removing items from the a collection you should to iterate in reverse to ensure you don't skip over any items:

private void btnRemoveCommand_Click(object sender, EventArgs e)
{
    for (int i = listBox1.SelectedItems.Count; i >= 0; i--)
    {
        using (var connection = new SqlConnection(connectionString))
        using (var command = new SqlCommand("DELETE FROM Commands WHERE commandName = @Command;", connection))
        {   
            connection.Open();
            //Add parameter with Add method - you may need to address the data type
            command.Parameters.Add("@Command", SqlDbType.VarChar, 50).Value = listBox1.SelectedItems[i];
            command.ExecuteNonQuery();
        }
        listBox1.Items.Remove(listBox1.SelectedItems[i]);   
    }
}

This is still not ideal, because if you have 1000 items, you are executing 1000 queries. My preferred way of doing this is with table valued parameters. The first step would be to create a table type in the database. I tend to use generic naming for ease of reuse:

CREATE TYPE dbo.ListOfString AS TABLE (Value NVARCHAR(MAX));

Then you can pass this type to your query to delete the records

private void btnRemoveCommand_Click(object sender, EventArgs e)
{
    var table = new DataTable();
    table.Columns.Add("Value", typeof(string));
    for (int i = listBox1.SelectedItems.Count; i >= 0; i--)
    {
        table.Rows.Add(new []{listBox1.SelectedItems[i]});
        listBox1.Items.Remove(listBox1.SelectedItems[i]);   
    }
    using (var connection = new SqlConnection(connectionString))
    using (var command = new SqlCommand("DELETE FROM Commands WHERE commandName IN (SELECT Value FROM @Commands);", connection)
    {   
        connection.Open();
        command.Parameters.Add(new SqlParameter("@Commands", SqlDbType.Structured) { Value = table, TypeName = "dbo.ListOfInt" });
        command.ExecuteNonQuery();
    }
}

Now you send a single command to the database, which is more efficient than sending multiple commands.

answered Jan 30, 2018 at 9:48

GarethD

70k11 gold badges90 silver badges130 bronze badges

9 Comments

wille480 Over a year ago

I see , so you are basicly creating a datatable to store all of it in there and send that as a query instead of sending a query for each and every single commandName pretty much? Ye i will be using using disposables! just did this quicky just to get it working, have disposable in some other event handlers , will switch it over^^

GarethD Over a year ago

Yes, exactly. If you are deleting a small number of items you will most likely find there is little or no difference, but with a lot of items you should see an increase in performance with just issuing a single query.

wille480 Over a year ago

oh oki got it! What do you mean? my database table is called " Commands " thats the table im refering to in the sql query ? ^^

GarethD Over a year ago

@TraxX If you mean can you use table valued parameters for these commands then yes.

GarethD Over a year ago

In object explorer under {Database} > Programmability > Types > User-Defined Table Types

|

Collectives™ on Stack Overflow

Invalid column name? c# sql

2 Answers 2

3 Comments

9 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

9 Comments

Your Answer

Sign up or log in

Post as a guest

Related