tags:

views:

172

answers:

4

when i compile the following code , "Conversion failed when converting datetime from character string" exception raises , what is wrong with that ?

code :

DateTime after3Dyas = DateTime.Now.AddDays(3);

        try
        {
            Con.Open();
            SqlCommand Command = Con.CreateCommand();
            Command.CommandText = "Select * from Forcast Where City='" + city + "' And Date between '" + DateTime.Now.Date + "' and '" + after3Dyas.Date + "'";

            SqlDataReader thisReader = Command.ExecuteReader();

            int i=0;
            while (thisReader.Read())
            {
                //do something
                i++;

            }

            thisReader.Close();
+1  A: 

The database is trying to convert the value from whatever DateTime.ToString is giving you... do you really want to trust that .NET on your calling machine and SQL Server use exactly the same format? That sounds brittle to me.

Avoid this by not putting the value into the SQL directly in the first place - use a parameterized query. This not only avoids conversion issues, but also (equally importantly) avoids SQL injection attacks.

Sample code:

DateTime start = DateTime.Now;
DateTime end = start.AddDays(3);
string sql = @"
SELECT * FROM Forecast
WHERE City = @City AND Date BETWEEN @StartDate AND @EndDate";

// Don't forget to close this somewhere. Why not create a new connection
// and dispose it?
Con.Open();
using (SqlCommand command = new SqlCommand(sql, Con))
{
    command.Parameters.Add("@City", SqlDbType.NVarChar).Value = city;
    command.Parameters.Add("@StartDate", SqlDbType.DateTime).Value = start;
    command.Parameters.Add("@EndDate", SqlDbType.DateTime).Value = end;
    using (SqlDataReader reader = command.ExecuteReader())
    {
        int i = 0;
        while (reader.Read())
        {
            //do something
            i++;
        }
    }
}
Jon Skeet
would you please tell me more ?!
persian Dev
@persian Dev: I've edited some code into the answer. Please look at that and read the link to the `Parameters` property for some more details. Any good book on .NET database access should cover parameterised queries comprehensively.
Jon Skeet
A: 

Try the format below instead :

DateTime.Now.ToString("yyyy-MM-dd")

But I strongly advice you to use parameters, because of security issues :

Command.CommandText = 
     "Select * from Forcast Where City=@City And Date between @StartDate and @EndDate";

SqlParameter city = new SqlParameter("@City", SqlDbType.VarChar, 10);
city.Value = yourCityValue;
Command.Parameters.Add(city);

SqlParameter startDate = new SqlParameter("@StartDate", SqlDbType.DateTime);
startDate.Value = yourStartDate;
Command.Parameters.Add(startDate);

SqlParameter endDate = new SqlParameter("@EndDate", SqlDbType.DateTime);
endDate.Value = yourEndDate;
Command.Parameters.Add(endDate);
Canavar
you mean using Parameters Property Of SqlCommand ?
persian Dev
@persian Dev : Yes, exactly.
Canavar
A: 
  1. You should use parametrized query.
  2. If you don't want to use parametrized query, use CONVERT function:

    "Select * from Forcast Where City='" + city + "' And Date = CONVERT(DATETIME,'" + DateTime.Now.ToString("yyyy-MM-dd") + "',120)

CONVERT(Datetime,'2009-12-25',120) converts varchar type to datetime type with specified format. It will also help with sql injection, but parameters are better solution.

LukLed
A: 

You should use parameterised queries whenever possible. There are several reasons such as:

  1. You will avoid sql injection attacks.
  2. Execution plans for parameterised queries will be cached by sql server so you will get better performance when executing the same query with different parameter values.
  3. You will avoid need to escape string parameters.

See the following article for more details: http://www.codeproject.com/KB/database/SqlInjectionAttacks.aspx

Giorgi