dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.68k stars 3.17k forks source link

SQL for string-updates sometimes uses nvarchar(450) if a string field is used in an index (not key) #8322

Closed iJungleboy closed 7 years ago

iJungleboy commented 7 years ago

Entity Framework Core auto-generates SQL-Statements to cause updates to happen. When a string-value is used as a key, it will by default use nvarchar(450) instead of nvarchar(max).

The problem is that this also happens in a few (but not all) code generated for string values which are in an index (instead of a key). Specifically, the temporary table which is used in the SQL for merge operations will incorrectly assume an nvarchar(450) and then fail when trying to place the initial nvarchar(max) value in this temp table.

The exception will then look be: SqlException: String or binary data would be truncated. The full exception shown is:

Microsoft.EntityFrameworkCore.DbUpdateException occurred
  HResult=0x80131500
  Message=An error occurred while updating the entries. See the inner exception for details.
  Source=Microsoft.EntityFrameworkCore.Relational
  StackTrace:
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(Tuple`2 parameters)
   at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](Func`2 operation, Func`2 verifySucceeded, TState state)
   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, Func`2 operation, TState state)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IReadOnlyList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at ToSic.Eav.Persistence.Efc.Models.EavDbContext.SaveChangesAndLogEverything(Boolean acceptAllChangesOnSuccess) in C:\Projects\eav-server\ToSic.Eav.Persistence.EFC11\ModelsEnhancements\EavDbContext.cs:line 39

Inner Exception 1:
SqlException: String or binary data would be truncated.
The statement has been terminated.

The SQL which was sent to the SQL Server was the following:

exec sp_executesql N'SET NOCOUNT ON;
DECLARE @toInsert0 TABLE ([AttributeID] int, [ChangeLogCreated] int, [ChangeLogDeleted] int, [ChangeLogModified] int, [EntityID] int, [Value] nvarchar(450), [_Position] [int]);
INSERT INTO @toInsert0
VALUES (@p0, @p1, @p2, @p3, @p4, @p5, 0),
(@p6, @p7, @p8, @p9, @p10, @p11, 1);

DECLARE @inserted0 TABLE ([ValueID] int, [_Position] [int]);
MERGE [ToSIC_EAV_Values] USING @toInsert0 AS i ON 1=0
WHEN NOT MATCHED THEN
INSERT ([AttributeID], [ChangeLogCreated], [ChangeLogDeleted], [ChangeLogModified], [EntityID], [Value])
VALUES (i.[AttributeID], i.[ChangeLogCreated], i.[ChangeLogDeleted], i.[ChangeLogModified], i.[EntityID], i.[Value])
OUTPUT INSERTED.[ValueID], i._Position
INTO @inserted0;

SELECT [t].[ValueID] FROM [ToSIC_EAV_Values] t
INNER JOIN @inserted0 i ON ([t].[ValueID] = [i].[ValueID])
ORDER BY [i].[_Position];

',N'@p0 int,@p1 int,@p2 int,@p3 int,@p4 int,@p5 nvarchar(max) ,@p6 int,@p7 int,@p8 int,@p9 int,@p10 int,@p11 nvarchar(450)',@p0=5757,@p1=15407,@p2=NULL,@p3=NULL,@p4=21419,@p5=N'<p><em>You can&nbsp;quickly change / remove this text&nbsp;once you''ve figured out how to work with the tiles.&nbsp;</em></p>
<p>This simple&nbsp;app let''s&nbsp;you choose icons to create nice tiles. The initial output is&nbsp;optimized for&nbsp;bootstrap skins.&nbsp;Your next steps are probably:</p>
<ol>
<li>Edit / add tiles (hover over the tiles and use the toolbar like the <i class="fa fa-pencil"></i>)</li>
<li>Write something else here or change the default look of <em>all</em> tiles in this group&nbsp;(just hover over this text and click <i class="fa fa-pencil"></i> in the toolbar)</li>
<li>Change the appearence of one&nbsp;icon (<i class="fa fa-pencil"></i> then change the presentation-settings <i class="fa fa-toggle-off"></i> at the bottom of the form)</li>
<li>Modify the HTML template as you wish&nbsp;(click on the <i class="fa fa-ellipsis-h"></i> till you get <i class="fa fa-code"></i>)</li>
</ol>',@p6=5756,@p7=15407,@p8=NULL,@p9=NULL,@p10=21419,@p11=N'Content Tiles App'

As you can see, the @p5 is correctly treated as an nvarchar(max) but the temporary table @toInsert0 creates a column [Value] which is cast as nvarchar(450) - triggering this problem.

In my case I had a DB first model which simply imported the indexes into the OnModelCreating ModelBuilder on my DbContext resulting in this index being managed:

entity.HasIndex(e => new { e.Value, e.ChangeLogCreated, e.EntityId, e.ChangeLogDeleted, e.AttributeId, e.ValueId })
   .HasName("IX_EAV_Values2");

When I commented out the information that e.Value is part of the index, everything worked flawlessly:

// 2017-04-28 disabled e.Value in this index - it also seems to affect sql queries to 450 chars on that field!
entity.HasIndex(e => new { /*e.Value,*/ e.ChangeLogCreated, e.EntityId, e.ChangeLogDeleted, e.AttributeId, e.ValueId })
    .HasName("IX_EAV_Values2");

It's important to note that many changes to this data type were not affected/hurt by the index-information. Just some very specific ones caused this problem.

Steps to reproduce

My code is too complex to use when reproducing, sorry. But I hope the full explanation above clarifies everything.

Further technical details

EF Core version: 1.1.1 Database Provider: Microsoft.EntityFrameworkCore.SqlServer Operating system: Windows 10 Dev environment, Windows 2012 R2 Web Server, IIS IDE: Visual Studio 2017

ajcvickers commented 7 years ago

Note for triage: this looks like another manifestation of #6835

/cc @divega @AndriySvyryd

ajcvickers commented 7 years ago

@iJungleboy What is the actual column type for Value?

ajcvickers commented 7 years ago

@iJungleboy Also, does the index IX_EAV_Values2 containing the column actually exist in the database, or is it only defined in EF?

AndriySvyryd commented 7 years ago

@AndriySvyryd Try using nvarchar(max) for all TVV columns in SQL Server 2008

iJungleboy commented 7 years ago

@ajcvickers The real type is nvarchar(max), and no, it was not in the DB index at all, that must be a minor bug in the DB-First model generation.

@AndriySvyryd As noted the column is already nvarchar(max) and the SQL version is 2012

ajcvickers commented 7 years ago

@iJungleboy Would it be possible to get a repro that caused this index to get reverse engineered? Hopefully, if you can post the table definitions we can do it from there.

iJungleboy commented 7 years ago

No problem - it's part of an open source EAV solution https://github.com/2sic/eav-server for the .net CMS DNN.

Here's the table definition, I assume it's complete but tell me if I missed something.

GO

/****** Object:  Table [dbo].[ToSIC_EAV_Values]    Script Date: 02.05.2017 13:00:28 ******/
SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE TABLE [dbo].[ToSIC_EAV_Values](
    [ValueID] [int] IDENTITY(1,1) NOT NULL,
    [EntityID] [int] NOT NULL,
    [AttributeID] [int] NOT NULL,
    [Value] [nvarchar](max) NOT NULL,
    [ChangeLogCreated] [int] NOT NULL,
    [ChangeLogDeleted] [int] NULL,
    [ChangeLogModified] [int] NULL,
 CONSTRAINT [PK_ToSIC_EAV_Values] PRIMARY KEY CLUSTERED 
(
    [ValueID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

GO

ALTER TABLE [dbo].[ToSIC_EAV_Values]  WITH CHECK ADD  CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_Attributes] FOREIGN KEY([AttributeID])
REFERENCES [dbo].[ToSIC_EAV_Attributes] ([AttributeID])
ON DELETE CASCADE
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values] CHECK CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_Attributes]
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values]  WITH CHECK ADD  CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_ChangeLogCreated] FOREIGN KEY([ChangeLogCreated])
REFERENCES [dbo].[ToSIC_EAV_ChangeLog] ([ChangeID])
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values] CHECK CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_ChangeLogCreated]
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values]  WITH CHECK ADD  CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_ChangeLogDeleted] FOREIGN KEY([ChangeLogDeleted])
REFERENCES [dbo].[ToSIC_EAV_ChangeLog] ([ChangeID])
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values] CHECK CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_ChangeLogDeleted]
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values]  WITH CHECK ADD  CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_ChangeLogModified] FOREIGN KEY([ChangeLogModified])
REFERENCES [dbo].[ToSIC_EAV_ChangeLog] ([ChangeID])
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values] CHECK CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_ChangeLogModified]
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values]  WITH CHECK ADD  CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_Entities] FOREIGN KEY([EntityID])
REFERENCES [dbo].[ToSIC_EAV_Entities] ([EntityID])
GO

ALTER TABLE [dbo].[ToSIC_EAV_Values] CHECK CONSTRAINT [FK_ToSIC_EAV_Values_ToSIC_EAV_Entities]
GO
ajcvickers commented 7 years ago

@iJungleboy Thanks very much for posting that so quickly. Hopefuly quick follow-up question: do you know if any other table references the "Value" column in, for example, a foreign key constraint?

iJungleboy commented 7 years ago

I don't think so, but I've included the full SQL to create the tables and stored procedures which are in use. eav-db.sql.txt