jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
861 stars 145 forks source link

6.2.7 introduces a breaking change with datetime TVP #386

Closed roryprimrose closed 6 years ago

roryprimrose commented 6 years ago

Describe the bug

Updating from 6.2.6 to 6.2.7 breaks usage of existing datetime fields in TVP.

Steps to reproduce

Send data to a procedure using a TVP that defines a datetime column.

Expected behavior

Procedure should execute successfully without unexpected exceptions.

Error thrown is

System.ArgumentException
Metadata for field 'LastSentForPeriodEnding' of record '2' did not match the original record's metadata.
   at System.Data.SqlClient.TdsParser.TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, Int32 timeout, Boolean inSchema, SqlNotificationRequest notificationRequest, TdsParserStateObject stateObj, Boolean isCommandProc, Boolean sync, TaskCompletionSource`1 completion, Int32 startRpc, Int32 startParam)
   at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry)
   at System.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(TaskCompletionSource`1 completion, String methodName, Boolean sendToPipe, Int32 timeout, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry)
   at System.Data.SqlClient.SqlCommand.ExecuteNonQuery()
   at Insight.Database.DBConnectionExtensions.<>c__DisplayClass164_0.<Execute>b__1(IDbCommand _, IDataReader __)
   at Insight.Database.DBConnectionExtensions.ExecuteAndAutoClose[T](IDbConnection connection, Func`2 getCommand, Func`3 translate, CommandBehavior commandBehavior)
   at Insight.Database.DBConnectionExtensions.Execute(IDbConnection connection, String sql, Object parameters, CommandType commandType, Boolean closeConnection, Nullable`1 commandTimeout, IDbTransaction transaction, Object outputParameters)

My guess is that this occurs because the previous mapping of data types replaced datetime with datetime2 instead of adding an additional mapping. See https://github.com/jonwagner/Insight.Database/commit/9dcb1351fc95e9fae931d1c70ab08b72ca970ef8#diff-ae7ae9df5ecc6ab21f91b989ed71b04eR38

jonwagner commented 6 years ago

What version of sql server are you using?

And to make it super-awesome, I let my editor reformat the files :(

jonwagner commented 6 years ago

I added a test case that shows that datetime2 can replace datetime in at least some TVP queries. Other than the sql server version, can you give me some more context on the table type and the query?

roryprimrose commented 6 years ago

Hi Jon, great to hear from you.

I'm using SQL Developer x64 version 14.0.2002.14

The user defined type is

CREATE TYPE [dbo].[SentSubscriptionReport] AS TABLE(
    [SubscriptionId] [uniqueidentifier] NOT NULL,
    [Email] [nvarchar](256) NOT NULL,
    [LastSentForPeriodEnding] [datetime2](7) NOT NULL
)
GO

The procedure definition for using this is

CREATE PROCEDURE [dbo].[CompleteSubscriptionReportBatch]

    @BatchId UNIQUEIDENTIFIER,
    @Items [SentSubscriptionReport] READONLY

AS

GO

In my scenario it is not a great issue as I am able to update my database to support the changes made in 6.2.7. The reason for raising this issue is that it might cause a breaking change for someone else where they are not able to change their database implementation.

jonwagner commented 6 years ago

Yes, I hate breaking changes too.

That type/query is similar to the one I’m using and I’m not getting the same result. How are you calling the proc? Through an interface?

On Nov 15, 2018, at 5:55 PM, Rory Primrose notifications@github.com<mailto:notifications@github.com> wrote:

Hi Jon, great to hear from you.

I'm using SQL Developer x64 version 14.0.2002.14

The user defined type is

CREATE TYPE [dbo].[SentSubscriptionReport] AS TABLE( [SubscriptionId] [uniqueidentifier] NOT NULL, [Email] nvarchar NOT NULL, [LastSentForPeriodEnding] datetime2 NOT NULL ) GO

The procedure definition for using this is

CREATE PROCEDURE [dbo].[CompleteSubscriptionReportBatch]

    @BatchId UNIQUEIDENTIFIER,
    @Items [SentSubscriptionReport] READONLY

AS

GO

In my scenario it is not a great issue as I am able to update my database to support the changes made in 6.2.7. The reason for raising this issue is that it might cause a breaking change for someone else where they are not able to change their database implementation.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/issues/386#issuecomment-439220349, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABk3dDLY6cdIORzfQeRQuUzEWCPVQkZ5ks5uvfDjgaJpZM4YfDEV.

roryprimrose commented 6 years ago

I hit the failure from a database integration test which uses an auto-implemented DAL. Here are the relevant parts.

var store = Config.Current.Worker.AsParallel<ISubscriptionReportStore>();

store.CompleteSubscriptionReportBatch(batchId, items);
public interface ISubscriptionReportStore
{
    void CompleteSubscriptionReportBatch(Guid batchId, IEnumerable<SentSubscriptionReport> items);
}
public class SentSubscriptionReport
{
    public string Email { get; set; }

    public DateTime LastSentForPeriodEnding { get; set; }

    public Guid SubscriptionId { get; set; }
}
NextStalker commented 6 years ago

Hi Jon. I have the similar error as Rory. It reproduce when list contains more than one item.

ellerbus commented 6 years ago

I'm having a similar issue, and don't have any control over the SQL Server instance setup. Works in 6.2.6 and breaks in 6.2.7 with the following error message if my list contains more than one item. I have confirmed that all C# records before calling the SQL sproc have been set to the current date/time. Hopefully this helps in some way track down the issue. Thanks in advance!

System.ArgumentException
  HResult=0x80070057
  Message=Metadata for field 'created_at' of record '2' did not match the original record's metadata.
  Source=System.Data.SqlClient
  StackTrace:
   at Microsoft.SqlServer.Server.ValueUtilsSmi.SetIEnumerableOfSqlDataRecord_Unchecked(SmiEventSink_Default sink, SmiTypedGetterSetter setters, Int32 ordinal, SmiMetaData metaData, IEnumerable`1 value, ParameterPeekAheadValue peekAhead)
   at Microsoft.SqlServer.Server.ValueUtilsSmi.SetCompatibleValueV200(SmiEventSink_Default sink, SmiTypedGetterSetter setters, Int32 ordinal, SmiMetaData metaData, Object value, ExtendedClrTypeCode typeCode, Int32 offset, Int32 length, ParameterPeekAheadValue peekAhead)
   at System.Data.SqlClient.TdsParser.WriteSmiParameter(SqlParameter param, Int32 paramIndex, Boolean sendDefault, TdsParserStateObject stateObj)
   at System.Data.SqlClient.TdsParser.TdsExecuteRPC(_SqlRPC[] rpcArray, Int32 timeout, Boolean inSchema, SqlNotificationRequest notificationRequest, TdsParserStateObject stateObj, Boolean isCommandProc, Boolean sync, TaskCompletionSource`1 completion, Int32 startRpc, Int32 startParam)
   at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, SqlDataReader ds)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean asyncWrite, String method)
   at System.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(TaskCompletionSource`1 completion, Boolean sendToPipe, Int32 timeout, Boolean asyncWrite, String methodName)
   at System.Data.SqlClient.SqlCommand.ExecuteNonQuery()
   at Insight.Database.DBConnectionExtensions.<>c__DisplayClass164_0.<Execute>b__1(IDbCommand _, IDataReader __)
   at Insight.Database.DBConnectionExtensions.ExecuteAndAutoClose[T](IDbConnection connection, Func`2 getCommand, Func`3 translate, CommandBehavior commandBehavior)
   at Insight.Database.DBConnectionExtensions.Execute(IDbConnection connection, String sql, Object parameters, CommandType commandType, Boolean closeConnection, Nullable`1 commandTimeout, IDbTransaction transaction, Object outputParameters)
   at My.BusinessRules.Repositories.IArticleTagRepository645b036b-4b00-427c-9987-547f476b0b26.SaveArticleTags(Int32 article_id, IEnumerable`1 items)
   at My.BusinessRules.Services.ArticleTagService.SaveArticleTags(Int32 articleId, IEnumerable`1 tags) in C:\Source\My\My.BusinessRules\Services\ArticleTagService.cs:line 106
   at My.WebApp.Controllers.ArticleTagsController.AddArticleTag(Int32 articleId, Int32 tagId) in C:\Source\My\My.WebApp\Controllers\ArticleTagsController.cs:line 49
   at Microsoft.Extensions.Internal.ObjectMethodExecutor.Execute(Object target, Object[] parameters)
   at Microsoft.AspNetCore.Mvc.Internal.ActionMethodExecutor.SyncObjectResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeActionMethodAsync>d__12.MoveNext()
public class ArticleTag
(
    [Column("article_id")] public int ArticleId { get; set; }
    [Column("tag_id")] public int TagId { get; set; }
    [Column("created_at"] public DateTime CreatedAt { get; set; }
    [Column("modified_at"] public DateTime? ModifiedAt { get; set; }
)
-- table defn
create table dbo.ArticleTag
(
    article_id                  int not null, -- PK
    tag_id                  int not null, -- PK
    created_at              datetime not null,
    modified_at             datetime null,
)

-- table type defn
create type dbo.ArticleTagTableType as table
(
    article_id              int,
    tag_id              int,
    created_at          datetime,
    modified_at         datetime
)

-- sproc header
create procedure dbo.ArticleTagUpsertMany
    @items dbo.ArticleTagTableType readonly
as
jonwagner commented 6 years ago

@NextStalker gets the gold star on this one. For some reason, it succeeds on the first item and fails on the second. This is fixed in 6.2.8 (coming soon).