0

I have the following code:

public static void dbInfoInsert(int ID)
{
    try
    {
        SqlConnection sqlCon = new SqlConnection(@"Data Source = (local); Initial Catalog = myDB; Integrated Security = True;");
        sqlCon.Open();

        SqlCommand insert = new SqlCommand
            {
                CommandText = string.Format("INSERT INTO [dbo.Food] ([FoodID], [FoodName], [FoodPrice], [FoodDescription]) VALUES ({0}, {1}, {2}, {3})", "T001", "FoodName", 23, "Food"),
                Connection = sqlCon
            };

        insert.ExecuteNonQuery();

        Console.Clear();
        Console.WriteLine("SUCCESS");
        Console.ReadKey();

        sqlCon.Close();
    }
    // In case connection to Microsoft SQL fails
    catch (SqlException e)
    {
        Console.WriteLine(e.ToString());
        Console.ReadKey();
    }
}

The error says that I have an Invalid column name 'T001', but that isn't my column. Am I doing something wrong here? In my database which name is myDB, I have a dbo.Food table which contains the following columns:

  • FoodID varchar(10)
  • FoodName varchar(100)
  • FoodPrice money
  • FoodDescription varchar(1000)
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Richard
  • 7,037
  • 2
  • 23
  • 76
  • 1
    I would highly recommend investigating the `SqlParameter` class as the way to pass data to your SQL Server. However if you wish to get your current code working the issue is because strings need to be quoted using single quotes, so `{0}` should actually be `'{0}'`. – Dale K Jan 01 '19 at 10:14
  • Oh, so all of those `{0}`, `{1}` etc. must always be enclosed with a single quotation mark? I didn't need to do that when I'm `Console.Write`-ing data and passing the value of some variables, though. Okay, I'll try to look for that one up, thanks for the response! – Richard Jan 01 '19 at 10:34
  • Strings and dates do, numbers don't. – Dale K Jan 01 '19 at 10:35
  • @DaleBurrell OHH! I remember now! When I wanna insert things in SQL Query, I need to use that `'` with strings, hence that. Okay, I forgot about that piece of info as I'm working with C# now xD Thanks once again! – Richard Jan 01 '19 at 10:37
  • @Dale Burrell : Single quotes convert a date to a string which is very dangerous. – jdweng Jan 01 '19 at 11:37

1 Answers1

1

You should always stick to SqlParamter to avoid Sql Injection.Additionally, it would also help you avoid mistakes like missing a ', as have happened without code.

    string commandText = @"INSERT INTO [dbo.Food] ([FoodID], [FoodName], [FoodPrice], [FoodDescription]) VALUES (@param1, @param2, @param3, @param4)";

        using (SqlConnection connection = new SqlConnection(connectionString))
        {
            SqlCommand cmd = new SqlCommand(sql,connection);
            cmd.Parameters.Add("@param1", SqlDbType.Varchar,10).value = "T001";  
            cmd.Parameters.Add("@param2", SqlDbType.Varchar, 100).value = "FoodName";
            cmd.Parameters.Add("@param3", SqlDbType.Money).value = 23;
            cmd.Parameters.Add("@param4", SqlDbType.Varchar, 100).value = "Food";
            cmd.CommandType = CommandType.Text;
            cmd.ExecuteNonQuery();
        }

Though not advisable, if you need to get your current code working, please wrap your varchar parameters with "'".

CommandText = string.Format("INSERT INTO [dbo.Food] ([FoodID], [FoodName], [FoodPrice], [FoodDescription]) VALUES ('{0}', '{1}', {2}, '{3}')", "T001", "FoodName", 23, "Food")
Anu Viswan
  • 17,797
  • 2
  • 22
  • 51
  • Thanks for the response! What does SQL Injection mean, though? And why do I need to use single quotation mark? When I'm `Console.Write`-ing and passing variable values to said `Console.Write`, I do not need to enclose `{0}` with a single quotation mark. – Richard Jan 01 '19 at 10:35
  • 1
    I believe you already got answer for why quotes is needed. You could read more on Sql Injection here https://www.cisco.com/c/en/us/about/security-center/sql-injection.html – Anu Viswan Jan 01 '19 at 10:51
  • Okay thanks! One more question, why do you use `using`? Mine seems to work perfectly fine? I seem to have found it here! https://stackoverflow.com/questions/75401/what-are-the-uses-of-using-in-c-sharp That means that I dont' have to manually do `sqlCon.Close()` in my case, right? – Richard Jan 01 '19 at 11:00
  • It ensures that IDisposable.Dispose Method is called, even if an exceptions occurs within the using block. In fact, this is equavalent of using try-catch and calling dispose in finally. – Anu Viswan Jan 01 '19 at 11:06
  • @WealthyPlayer and Anu, it's worth noting that SqlCommand is also IDisposable so should also be in a `using` block. – Richardissimo Jan 01 '19 at 23:30
  • @Richardissimo Okay, thanks! I assume that's the best practice with IDisposable items as I may occasionally forget to `Dispose()` said object (or in this case, `Close()`). Thank you for the comment :-) – Richard Jan 01 '19 at 23:43
  • @WealthyPlayer yes, it's best practice because anything which is IDisposable should have Dispose called, and it's the simplest way of writing it, and also the best because (as Anu mentioned) it saves you from having to write `try...finally` (i.e. it works in scenarios where exceptions are thrown). – Richardissimo Jan 02 '19 at 17:37