Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
iJungleboy opened this issue Apr 28, 2017 · 9 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@iJungleboy
Copy link

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
Copy link
Contributor

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

/cc @divega @AndriySvyryd

@ajcvickers
Copy link
Contributor

@iJungleboy What is the actual column type for Value?

@ajcvickers
Copy link
Contributor

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

@AndriySvyryd
Copy link
Member

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

@iJungleboy
Copy link
Author

@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
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Author

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

AndriySvyryd added a commit that referenced this issue Jun 9, 2017
AndriySvyryd added a commit that referenced this issue Jun 9, 2017
@AndriySvyryd AndriySvyryd removed their assignment Jun 9, 2017
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

3 participants