Add Support for DateOnly & TimeOnly types on SqlDataRecord

[dckorben]

Since DateOnly and TimeOnly were added to NET 6, there has been some limited changes to the API to add direct paths for these new types.
 
I'd like to see SetDate and SetTime added to SqlDataRecord to bypass the need to instantiate a DateTime each time for columns with SqlDbType.Date and SqlDbType.Time. Or alternatively, overloads to SetDateTime which take these types directly.

Current workaround is to convert to a full DateTime via .ToDateTime(TimeOnly.MinValue) each time.

[ErikEJ]

Could you provide a code sample?

[dckorben]

The general use is, populate a structured table variable that is sent to a stored procedure, but the SqlDataRecord is reused for each row by convention as to not duplicate the data many times for each row as it is being streamed as it would with say a DataTable. Let me know if this is an insufficient repo. What is handy is the data can be changing up until the moment it is streamed out. I have not used TimeOnly in this way yet but makes sense to include it.

using Microsoft.Data.SqlClient;
using Microsoft.Data.SqlClient.Server;
using System.Collections.Generic;
using System.Data;

#if NET6_0_OR_GREATER
using DateType = System.DateOnly;
#else
using DateType = System.DateTime;
#endif

public class Record
{
    public string Source { get; init; }
    public DateTime UpdateStamp { get; init; }
    public DateType AsOfDate { get; init; }
}

public class RecordStreamer<T>
    where T : Record
{
    const string SourceKey = @"Source";
    readonly int SourceIndex;

    const string UpdateStampKey = @"UpdateStamp";
    readonly int UpdateStampIndex;

    const string AsOfDateKey = @"AsOfDate";
    readonly int AsOfDateIndex;

    SqlDataRecord SqlRecord { get; init; }

    public RecordStreamer()
    {
        //Create Column Metadata
        var columns = new List<SqlMetaData>();

        var sourceColumn = new SqlMetaData(SourceKey, SqlDbType.NVarChar, 128);
        columns.Add(sourceColumn);
        SourceIndex = columns.Count - 1;

        var updateStampColumn = new SqlMetaData(UpdateStampKey, SqlDbType.DateTime);
        columns.Add(updateStampColumn);
        UpdateStampIndex = columns.Count - 1;

        var asOfDateColumn = new SqlMetaData(AsOfDateKey, SqlDbType.Date); //DbType doesn't change since it is a valid SQL Type
        columns.Add(asOfDateColumn);
        AsOfDateIndex = columns.Count - 1;

        //Create SqlDataRecord
        var sqlRecord = new SqlDataRecord(columns.ToArray()); //Must this be an array?
        SqlRecord = sqlRecord;
    }

    public void Execute(SqlConnection connection, IEnumerable<T> data)
    {
        using var command = GetCommand(connection, data);
        Execute(command);
    }

    void Execute(SqlCommand command)
    {
        var connection = command.Connection;
        connection.Open();
        var impactedRows = command.ExecuteNonQuery();
        connection.Close();
    }

    SqlCommand GetCommand(SqlConnection connection, IEnumerable<T> data)
    {
        var command = new SqlCommand("[dbo].[pc_Audit]", connection)
        {
            CommandType = CommandType.StoredProcedure
        };
        _ = AddDataParam(command, data);
        return command;
    }

    SqlParameter AddDataParam(SqlCommand command, IEnumerable<T> data)
    {
        var parameter = new SqlParameter("@Data", SqlDbType.Structured)
        {
            Value = GetSqlDataRecords(data)
        };
        command.Parameters.Add(parameter);
        return parameter;
    }

    IEnumerable<SqlDataRecord> GetSqlDataRecords(IEnumerable<T> data)
    {
        foreach (var row in data)
        {
            if (SetSqlDataRecord(row))
                yield return SqlRecord;
        }
    }

    bool SetSqlDataRecord(T row)
    {
        SqlRecord.SetString(SourceIndex, row.Source);

        SqlRecord.SetDateTime(UpdateStampIndex, row.UpdateStamp);

        SqlRecord.SetValue(AsOfDateIndex , row.AsOfDate); //Fail: Boxes & Cast, though why isn't everything an overload?
        SqlRecord.SetDate(AsOfDateIndex , row.AsOfDate); //Invalid: This would make sense with Net6 and greater
        SqlRecord.SetDateTime(AsOfDateIndex , row.AsOfDate.ToDateTime(TimeOnly.MinValue)); //Workaround
        return true;
    }
}

[ErikEJ]

@dckorben Thanks, so you are really asking for two features:

• SqlRecord.SetValue(); SqlRecord.GetValue(); SqlRecord.GetSqlValue(); should be updated to support a DateOnly or TimeOnly value
• New APIs should be added to SqlRecord: SetDate and SetTime and possibly also GetDate and GetTime

Is that correctly understood?

[ErikEJ]

I cannot make this cal fail, does it only fail later?

SqlRecord.SetValue(AsOfDateIndex , row.AsOfDate);

[dckorben]

I cannot make this cal fail, does it only fail later?

SqlRecord.SetValue(AsOfDateIndex , row.AsOfDate);

Let me check this. I haven't tried the object methods in a long time. I've only used the strongly typed lately.
I've never had a need for the GetX paths but makes sense to support for completeness.

[dckorben]

SqlRecord.SetValue(AsOfDateIndex, row.AsOfDate);

Fails on execution. System.InvalidCastException: 'Specified cast is not valid.'

I guess the point here is a DateOnly or TimeOnly should work for SetValue for a column with the right metadata. It does not work when I try so really the workaround is the only viable way to get a DateOnly thru but I am really more interested in the strongly typed for the API.

CREATE TYPE [dbo].[tp_Test] AS TABLE (
    [Source]      NVARCHAR (128) NULL,
    [UpdateStamp] DATETIME2 (0)  NULL,
    [AsOfDate]    DATE           NULL);

CREATE PROCEDURE [dbo].[pc_Audit]
	 @Data [dbo].[tp_Test] READONLY
AS
	SELECT * FROM @Data
RETURN 0

[roji]

Just noting that SqlDataRecord implements IDataRecord; both are extremely old (I'd say mostly obsolete) types which haven't been updated for a very long time. For example, there's no way to generically read any type (in the same way that DbDataRecord has a generic GetFieldValue<T>). I'd advise against adding new APIs here - at most I'd look into making sure that SqlClient supports DateOnly/TimeOnly when the existing object-APIs are used.

But I'd also advise user to look at moving away from these largely obsolete types.

[ErikEJ]

I am not planning to add new APIs, but plan to prevent runtime errors.

[dckorben]

Just noting that SqlDataRecord implements IDataRecord; both are extremely old (I'd say mostly obsolete) types which haven't been updated for a very long time. For example, there's no way to generically read any type (in the same way that DbDataRecord has a generic GetFieldValue<T>). I'd advise against adding new APIs here - at most I'd look into making sure that SqlClient supports DateOnly/TimeOnly when the existing object-APIs are used.

But I'd also advise user to look at moving away from these largely obsolete types.

Too bad there isn't a modernized equiv, unless table types are going to be deprecated.

[cheenamalhotra]

As @roji mentioned, these are old design APIs, not heavily used today. If we see a lot of interest from the community on this issue, this can be considered in future.. but as of now this isn't on our priority list.

[ErikEJ]

@cheenamalhotra I plan to fix the TVP runtime issue

[ErikEJ]

@dckorben I am not able to repro with SqlClient 6 - could you please try that!

I think it might have been fixed in #2258

[dckorben]

It does work in the latest 6 prerelease.
Still think it's a shame to cast this as obsolete given the unbelievable performance that can be achieved with it.

[ErikEJ]

@dckorben Thanks for confirming.

I will let the team decide if anything else needs to be done here..

[roji]

Still think it's a shame to cast this as obsolete given the unbelievable performance that can be achieved with it.

AFAIK there's nothing you can do with SqlDataRecord - and no performance - that can't be achieved with SqlDataReader, which is fully, actively supported (and is what SqlDataRecord uses under the hood).

[ErikEJ]

@roji SqlDataRecord (or DataTable) must be used a TVP parameter - so it still has it's uses. (And SqlDataRecord is more performant than DataTable in this scenario)

[roji]

And SqlDataRecord is more performant than DataTable in this scenario

Is it really, in a significant way?

[roji]

Also, isn't it possible to use DbDataReader for TVP (docs)? I get that it's probably more work etc., but still.

[ErikEJ]

Is it really, in a significant way?

It was for me in a production use case, yes.

use DbDataReader

Forgot about that approach (needing a lot of code/efffort yes)

[dckorben]

Is it really, in a significant way?

It was for me in a production use case, yes.

Can confirm.

[benrr101]

@dckorben so, just to check in, this is working as expected in v6.0.1? If so, I think we can close this one :)

[ErikEJ]

@benrr101 yes, but no new APIs - I guess you are not planning that?

[dckorben]

@dckorben so, just to check in, this is working as expected in v6.0.1? If so, I think we can close this one :)

Yes, it is working but still the ask would be to add APIs and/or modernize this approach because of the performance gains that are possible. I can create a new issue for that, but from the tone of the thread, it looks like that would be a waste of time.