1
Vote

UpForGrabs: Insert fails if table has a DATETIME property as part of the key

description

Hi,

please looking at my stackoverflow question for detailed explanation.


http://stackoverflow.com/questions/22655842/entity-framework-insert-failed-with-date-column

Regards
Daniel

comments

divega wrote Apr 1, 2014 at 11:02 PM

Note for triage (edited):

Seems to be a legitimate issue although not a regression from either EF5 or EF6.

The root of the problem is a mismatch between a column type and the type of the corresponding parameter used in the commands we generate in the update pipeline for certain types and facets.

In this particular case a property of type System.DateTime is mapped to a DateTime column and the parameter generated for the INSERT command is of type SqlDbType.DateTime2.

The property is assigned an arbitrary DateTime value, e.g. DateTime.Now.

When we execute the INSERT statement the value stored in the DateTime2 parameter @0 will be silently truncated to make it fit in the StartDate column of type DateTime:
INSERT [dbo].[CalendarItems]([StartDate]) VALUES (@0) 
In the subsequent query to evaluate the number of rows affected, SQL Server will transparently upcast the DateTime column to DateTime2 to perform the comparison against the parameter and the values will not match because of the precision loss:
SELECT [Id] FROM [dbo].[CalendarItems] WHERE @@ROWCOUNT > 0 AND [StartDate] = @0
In the end we will receive a rows affected value of 0 and will throw an exception saying that the insert statement affected an unexpected number of rows.

I found that in In SqlProviderServices.CreateSqlParameter() we decide the type of the parameter only based on a C-space type usage in the insert command tree. We generate a parameter of SqlDbType.DateTime2 if supported by the database version for all properties of type DateTime. Looks like an oversight in the design because in some cases the store type might be different and in this particular case it causes loss of precision.
That said I haven’t found any other cases in which this can happen. Although bug #2036 (which is about DateTimeOffset properties as concurrency tokens) seems related I haven’t been able to confirm.

We can consider fixing it if we believe the cost is reasonable and we feel comfortable that it won't cause unintended breaking changes.
There are a few workarounds for this particular case:
  1. Use a surrogate key instead of a DateTime column as a key
  2. Make the type of the column DateTime2 (you can do this easily with a custom convention now)
As a side note, I investigated a few other concerns I had but they turned out to be negative:
  1. This seem only for some types for which we have a precision facet, e.g. it happens for DateTime and apparently for DateTimeOffset, but it doesn't happen for decimal (for keys of type decimal we generate parameters with the same precision as the column).
  2. This does not seem to affect other facets, e.g. I confirmed for string keys that are defined as non-Unicode we produce the right non-Unicode parameter, even in the update pipeline.

divega wrote Apr 1, 2014 at 11:15 PM

An additional workaround is to truncate the value of the System.DateTime property before saving changes.

scda0001 wrote Apr 1, 2014 at 11:25 PM

yes, truncating the time part is a working solution. But this should be only a workaround. This issue has to be fixed for future releases. I cannot see a breaking change (as far as I can evaluate this).

scda0001 wrote Apr 2, 2014 at 10:15 AM

Can this issue have any side effects with optimistic concurrency pattern too!? (I don't tested it yet)
http://msdn.microsoft.com/en-us/data/jj592904

If you don't use any timestamp column for concurrency check and instead compare all table columns, I would assume that this raise the same issue.

divega wrote Apr 2, 2014 at 11:39 AM

I believe it won"t have an effect on concurrency although I haven't tested all my assumptions: in concurrency control the value that needs to match is the original value if the column after a roundtrip. The original value has already been stored in a DATETIME column before and therefore it is already truncated. A DATETIME2 parameter should have no problem holding the value with a lesser precision and the comparison should match.

I suspect in #2036 the root cause is the opposite, i.e. a parameter cannot hold the whole precision of the column.

scda0001 wrote Apr 2, 2014 at 2:40 PM

You are right. For concurrency check original values are used, so no truncation occured.

RoMiller wrote Apr 3, 2014 at 9:24 PM

EF Team Triage: We agree that we should fix this issue. Given that this is not a regression from a previous release we are not going to take a fix in the 6.1.1 patch release. We're moving it to the Future release to reconsider for the next release. Also marking as UpForGrabs in case someone outside of the EF team wants to contribute the fix.