1

From within my C# app I'm calling a stored procedure with a TVP. A couple of columns are datetime. A call to the SP might look like:

declare @p1 dbo.MyTvp
insert into @p1 values('2020-03-19 00:00:01','2020-03-30 23:59:59')
exec MySp @criteria=@p1

The above code is automatically generated in C#. In the SP, the part handling the dates is:

declare @datefrom datetime;
---
SET @sql = CONCAT(@sql, ' AND date_from >= ''', @datefrom, '''');

SQL Server locale is German.

The above throws an error due to conversion from varchar to datetime. However, if the datetime values that I pass are formatted as follows:

declare @p1 dbo.MyTvp
insert into @p1 values('19.03.2020 00:00:01','30.03.2020 23:59:59')
exec MySp @criteria=@p1

The SP works fine.

The class used as a source is:

public class MyCriteria
{
    public DateTime DateFrom { get; set; }
}

And the table type is:

CREATE TYPE [dbo].[MyTvp] AS TABLE(
    [DateFrom] [datetime] NULL
)

I convert an instance of MyCriteria into a DataTable using an extension method, and then use Dapper to execute the SP:

var criteria = new List<MyCriteria>() { myCriteria }.ToDataTable();
return await conn.QueryAsync<SomeResult>(new CommandDefinition("MySp", new { criteria }, commandType: CommandType.StoredProcedure, cancellationToken: ct));

What I don't understand is at what stage does the conversion from datetime to varchar or DateTime to string occurs.

So how exactly do I need to convert the dates to get the SP to work? Should I do the conversion at the DB level or in my C# app?

EDIT

This is the extension method used to convert a class to a datatable so that it can be passed on as a TVP to the SP:

    public static DataTable ToDataTable<T>(this IEnumerable<T> items)
    {
        var dataTable = new DataTable(typeof(T).Name);

        //Get all the properties not marked with Ignore attribute
        var properties = typeof(T).GetProperties(BindingFlags.Public | BindingFlags.Instance)
                                  .Where(x => x.GetCustomAttributes(typeof(XmlIgnoreAttribute), false).Length == 0).ToList();

        //Set column names as property names
        foreach (var property in properties)
        {
            if (!property.PropertyType.IsEnum && !property.PropertyType.IsNullableEnum())
            {
                var type = property.PropertyType;

                //Check if type is Nullable like int?
                if (Nullable.GetUnderlyingType(type) != null) 
                    type = Nullable.GetUnderlyingType(type);

                dataTable.Columns.Add(property.Name, type);
            }
            else dataTable.Columns.Add(property.Name, typeof(int));
        }

        //Insert property values to datatable rows
        foreach (T item in items)
        {
            var values = new object[properties.Count];
            for (int i = 0; i < properties.Count; i++)
            {
                values[i] = properties[i].GetValue(item, null);
            }

            dataTable.Rows.Add(values);
        }

        return dataTable;
    }

EDIT 2

The problem is the SQL that is being generated by C#/Dapper which is used to populate the TVP passed to the SP. A simple test can be seen by doing the following:

DECLARE @test TABLE (
    [DateCol] datetime NOT NULL
);

INSERT INTO @test VALUES ('2020-02-19 00:00:01'); --doesnt work
INSERT INTO @test VALUES (CONVERT(datetime, '2020-02-19 00:00:01', 120)); --works

The CONVERT function returns the date in the same format as the first INSERT statement. However the first statement doesn't work.

Ivan-Mark Debono
  • 15,500
  • 29
  • 132
  • 263
  • is there any reason you can't just use `datetime` in the TVP? The answer to "how to format/parse a date with SQL Server" is usually "don't; just type the data correctly, and send it as typed data via parameters" (where parameters includes typed columns in the case of TVP) – Marc Gravell Mar 20 '20 at 10:31
  • 1
    btw: `SET @sql = CONCAT(@sql, ' AND date_from >= ''', @datefrom, '''');` is a SQL injection nightmare; if you are using `sp_executesql` (and you should be), you can pass typed parameters *into* your dynamic execs, avoiding issues of formatting/parsing and making SQLi impossible; alternatively; you could just try writing regular non-dynamic SQL based off the TVP? although unless the TVP usually has more than one row, I'm not sure why you're using a TVP in the first place, to be honest – Marc Gravell Mar 20 '20 at 10:32
  • @MarcGravell Both my TVP and my C# class have datetime (ie. DateTime) data types. The above is what gets generated by Dapper. So I'm avoid sql injection, formatting, parsing, etc...I'm using a TVP because I convert my class to a DataTable automatically before passing it to the SP. – Ivan-Mark Debono Mar 20 '20 at 10:34
  • I'd be interested to see that code, but that's probably not hugely related, but OK; the TVP has a datetime column? that's great! in that case, we're just talking about the `CONCAT` bit - is that right? now: does your TVP actually have multiple rows usually? or is the TVP always used with one row? it impacts what we need to do to parameterize it correctly – Marc Gravell Mar 20 '20 at 10:41
  • @MarcGravell In this particular case the TVP always has one row (so I just get TOP 1 into local variables (with the corresponding datatype)). I use the method with DataTable->TVP when calling SP's because I have other SP's where the TVP has multiple rows, and I want to keep the calling of SP from C# as standardized as possible. – Ivan-Mark Debono Mar 20 '20 at 10:45

1 Answers1

1

From discussion in the comments, it sounds like a: the data in the TVP is typed (datetime), and b: there is only one row in this case; that's great - it means we can simplify hugely; what we'd want to do here is pull the values from the TVP into locals, and just work with those. Now, based on @datefrom in the example code, it sounds like you've already done the first step, so all we need to do is fix how the dynamic SQL is composed and executed. In the question we have:

SET @sql = CONCAT(@sql, ' AND date_from >= ''', @datefrom, '''');

which is presumably followed later by:

EXEC (@sql);

Instead, we can parameterize the dynamic SQL:

SET @sql = @sql + ' AND date_from >= @datefrom ';

and pass the parameters into our dynamic SQL:

exec sp_executesql @sql, N'@datefrom datetime', @datefrom

The second parameter to sp_executesql gives the definition string for all the actual parameters, which come after it sequentially.

Now the code is entirely safe from SQL injection, and you don't have any string/date conversions to worry about.

Note that the parameter names don't need to be the same in the "inside" and "outside" parts, but they often are (for convenience and maintainability).

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • I changed the SP to use `sp_executesql` and I still get the conversion error!! – Ivan-Mark Debono Mar 20 '20 at 11:07
  • @Ivan-MarkDebono if the `date_from` column is `datetime`, and the `@datefrom` local/parameter is `datetime`, there is literally no conversion happening; I set up a dummy table locally and tested things - it should work fine if we make those assumptions. Beyond that, we'd need to see realistic code. Are you sure that everything is correctly typed here (including the TVP column)? – Marc Gravell Mar 20 '20 at 11:10
  • Everything is datetime/DateTime. All columns match too (if not I'd get errors). The date value being sent from C# to the db is still as is: '2020-02-19 00:00:01'. Is this correct? – Ivan-Mark Debono Mar 20 '20 at 11:15
  • @Ivan-MarkDebono you shouldn't be sending *strings* **at all**; can we perhaps look at the `var criteria = new List() { myCriteria }.ToDataTable();` step in more detail? What is the date in `MyCriteria`? is it `string`? or `DateTime`? hint: it should be `DateTime` – Marc Gravell Mar 20 '20 at 11:18
  • I'm definitely not sending strings. `MyCriteria` is a class that is a mapping of the TVP (names & datatypes).. If strings shouldn't be passed, then I suspect the `ToDataTable()` extension method is wrong. I've added it to the question. – Ivan-Mark Debono Mar 20 '20 at 11:23
  • @Ivan-MarkDebono are you sure that your data table is binding *in the right order*? the `DataTable` => TVP bridge is *positional*, not *nominal*; so : the order in which the columns get added and populated to your `DataTable` needs to line up with the TVP definition; is it possible that you're actually sending some other property as the date due to this positional vs nominal thing? – Marc Gravell Mar 20 '20 at 11:32
  • It matches 100%. I only have 2 `datetime` columns and when I pass in values from the class to the TVP, those values are in the right columns (however in the wrong format). Could it be related to english/german locales? – Ivan-Mark Debono Mar 20 '20 at 11:43
  • @Ivan-MarkDebono it shouldn't be, because *nothing here is being handled as strings*; I hate to say it, but a minimal *but complete* repro here might really help – Marc Gravell Mar 20 '20 at 11:51
  • The problem is not with the SP itself but how the SQL is being generated as shown in the first code-block of the question. This SQL is being generated by Dapper, right? It's the insert statement into the TVP that is causing the error. If I change the date value to '19.02.2020 00:00:01', the insert statement works fine. – Ivan-Mark Debono Mar 20 '20 at 19:34
  • @ivan no, that isn't code that exists anywhere; that's not how TVPs are actually sent. Presumably you're looking at profiler data? The profiler lies, to try and make things readable. But again, this is why I'm asking about a minimal but complete repro, so that we can investigate properly (I'm the primary Dapper maintainer) – Marc Gravell Mar 21 '20 at 02:57
  • I've created a simple repro with a local db. The error cannot be reproduced. It might be related to the SQL Server instance's local and it being on a separate machine (with different locale) – Ivan-Mark Debono Mar 21 '20 at 05:31
  • @Ivan-Mark I have a vested interest in understanding what is happening here, but ... I suspect that right now the key is managing to reproduce it; I'm very intrigued – Marc Gravell Mar 21 '20 at 10:52
  • I added the test code here: https://github.com/cryo75/TVPTest – Ivan-Mark Debono Mar 22 '20 at 14:04