jphellemons/CommandAsSql

date handling

Opened this issue · 6 comments

Do you feel the
retval = "'" + sp.Value.ToString().Replace("'", "''") + "'";
is correct for DateTime?

I'm getting localized Dates in Greek when I use it via CommandAsSql library.

In the library code it also uses later on sb.Append("'").Append(Convert.ToDateTime(dr[colIndex]).ToString("yyyy-MM-dd HH:mm")).Append("'");
for handling structure types which seems more logical to use everywhere

I mean the .ToString("yyyy-MM-dd HH:mm") - not sure about the DateTimeOffset and DateTime2, guess for those too

if you see
https://github.com/jeroenpot/SqlHelper/blob/master/Source/Mirabeau.MsSql.Library/SqlGenerator.cs

it has in GetParameterValue the following:

            case SqlDbType.Date:
            case SqlDbType.DateTime:
            case SqlDbType.DateTime2:
            case SqlDbType.DateTimeOffset:
                  var dateTime = ((DateTime)sqlParameter.Value)
                                                    .ToString("yyyy-MM-dd HH:mm:ss:fff", FormatCulture);
                 retval = string.Format(FormatCulture, "convert(datetime,'{0}', 121)", dateTime);
                  break;

so probably it is wrong as is currently in this library

also should check how they handle some of their other parameters differently in that method and compare with the implementation in this library to merge / keep the most types and use the correct implementation (not assuming theirs is always the correct one)

btw, they also use :ss:fff but that other code in this library that I had mentioned (which is similar to their version) doesn't include the :ss:fff - so not sure if one should add those two parts in the output date (or maybe the ss:fff means to not output if they're zero, don't remember)

also, not sure what this code of theirs does: string.Format(FormatCulture, "convert(datetime,'{0}', 121)", dateTime);

121 = yyyy-mm-dd hh:mi:ss.mmm

yep, that's the format I'd expect to see, but the code in the library currently just spits out the date in localized format cause of .ToString() instead of using .ToString("yyyy-MM-dd HH:mm"))

I mean the issue is at

return $"'{paramValue.ToString().Replace("'", "''")}'";

and it should do like in

sb.Append("'").Append(Convert.ToDateTime(dr[colIndex]).ToString("yyyy-MM-dd HH:mm")).Append("'");

that is:
return $"'{paramValue.ToString("yyyy-MM-dd HH:mm").Replace("'", "''")}'";
but not sure if that .Replace("'", "''") is needed, so maybe that should be removed

plus do that at a separate case statement for the following ones (now they were together with other cases in the switch at ParameterValueForSQL method):
case SqlDbType.Date:
case SqlDbType.DateTime:
case SqlDbType.DateTime2:
case SqlDbType.DateTimeOffset:

not sure for the Date one (maybe should leave that as is). Also I'm not familiar with DateTime2 and especially DateTimeOffset types, but for the DateTime one, should definitely use a separate case and format the date instead of spitting out a localized one

...the https://github.com/jeroenpot/SqlHelper/blob/master/Source/Mirabeau.MsSql.Library/SqlGenerator.cs I mentioned above does it for all those types, however what it does looks too complex to me (not sure of the rationale behind it)