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

Failing to sort owned types, when updating different combinations of fields of both parent and owned type at the same time #10568

Closed
lakeman opened this issue Dec 15, 2017 · 10 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

@lakeman
Copy link

lakeman commented Dec 15, 2017

I'm not sure I really understand this problem yet....

I created an owned type to wrap every enum value, so I could save string names instead of numeric values without repeating myself too much. (Side note, this still isn't ideal as it requires each user of any enum to be more careful in how they assign a value, since an implicit cast from T creates a new instance which doesn't seem to be handled...)

public class EnumWrapper<T> where T : struct // eg enum
{
	public EnumWrapper() : this(Default)
	{
	}
	public EnumWrapper(T value) {
		this.Value = value;
	}

	private static T Default = (T)Enum.GetValues(typeof(T)).GetValue(0);

	[NotMapped, Required]
	public T Value { get; set; }

	[StringLength(20), Required]
	public string _Raw { get { return Value.ToString(); } set { Value = Extensions.ParseEnum<T>(value, Default); } }
}

The model is built by calling this extension method for every use of this type;

public static PropertyBuilder<string> HasEnum<T, E>(this EntityTypeBuilder<T> builder, Expression<Func<T, EnumWrapper<E>>> navigationExpression) 
	where T : class 
	where E : struct
{
	var entityBuilder = builder.OwnsOne(navigationExpression);
	entityBuilder.Ignore(e => e.Value);
	return entityBuilder
		.Property(e => e._Raw)
		.HasColumnName(Database.ToSnakeCase(entityBuilder.OwnedEntityType.DefiningNavigationName))
		.HasMaxLength(20);
}

I can change multiple instances of this type at the same time, unless (I believe) two existing instances have been changed from the same before value to the same after value. Then I'm seeing this exception thrown;

 An exception occurred in the database while saving changes for context type '<DbContext>'.
 System.InvalidOperationException: Failed to compare two elements in the array. ---> System.InvalidOperationException: No backing field could be found for property 'TransactionId' of entity type 'EnumWrapper<TransactionStatus>' and the property does not have a getter.
    at Microsoft.EntityFrameworkCore.Metadata.Internal.ClrAccessorFactory`1.Create(PropertyInfo propertyInfo, IPropertyBase propertyBase)
    at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
    at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.ReadPropertyValue(IPropertyBase propertyBase)
    at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.get_Item(IPropertyBase propertyBase)
    at Microsoft.EntityFrameworkCore.Update.Internal.ModificationCommandComparer.Compare(ModificationCommand x, ModificationCommand y)
    at System.Collections.Generic.ArraySortHelper`1.InsertionSort(T[] keys, Int32 lo, Int32 hi, Comparison`1 comparer)
    at System.Collections.Generic.ArraySortHelper`1.IntrospectiveSort(T[] keys, Int32 left, Int32 length, Comparison`1 comparer)
    at System.Collections.Generic.ArraySortHelper`1.Sort(T[] keys, Int32 index, Int32 length, IComparer`1 comparer)
    --- End of inner exception stack trace ---
    at System.Collections.Generic.ArraySortHelper`1.Sort(T[] keys, Int32 index, Int32 length, IComparer`1 comparer)
    at System.Collections.Generic.List`1.Sort(Int32 index, Int32 count, IComparer`1 comparer)
    at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__8.MoveNext()
    at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__10.MoveNext()
@ajcvickers
Copy link
Contributor

@lakeman The exception says, No backing field could be found for property 'TransactionId' of entity type 'EnumWrapper<TransactionStatus>' and the property does not have a getter. However, I don't see a TransactionId property in the class--are you getting this specific exception with the code above, or is there more code that isn't posted above? It would be very useful to get a runnable project/solution or full code listing so that we can easily reproduce what you are seeing.

@lakeman
Copy link
Author

lakeman commented Dec 15, 2017

In this case the parent type is Transaction, with a key field of Id.

@ajcvickers
Copy link
Contributor

@lakeman I have not been able to repro this with just the code posted. Could you please post a runnable project/solution or full code listing so that we can easily reproduce what you are seeing?

@ajcvickers
Copy link
Contributor

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.

BTW this is a canned response and may have info or details that do not directly apply to this particular issue. While we'd like to spend the time to uniquely address every incoming issue, we get a lot traffic on the EF projects and that is not practical. To ensure we maximize the time we have to work on fixing bugs, implementing new features, etc. we use canned responses for common triage decisions.

@lakeman
Copy link
Author

lakeman commented Jan 11, 2018

I have been attempting to reproduce this, but I haven't been able to work out exactly what the pre-conditions are to produce a simpler test case.

Looking at ModificationCommandComparer.cs;
https://github.com/aspnet/EntityFrameworkCore/blob/fbaebcbfb57f245edfded68f0bad78f4978d8867/src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs#L41

The collection of ModificationCommand are being sorted by schema, then table name, and finally the primary key of the first element of each array. It looks like this method is assuming that both of these first elements are the same EntityType.

With the introduction of owned types, can these EntityType's be different? Should the sorting here compare entity types first, before assuming that they are the same?

@lakeman
Copy link
Author

lakeman commented Jan 11, 2018

I've managed to trigger this reliably;
https://github.com/lakeman/OwnedTypesException

@lakeman lakeman changed the title Failing to sort owned types? Failing to sort owned types, when updating different combinations of fields of both parent and owned type at the same time Jan 11, 2018
@ajcvickers ajcvickers reopened this Jan 11, 2018
@ajcvickers ajcvickers modified the milestones: 2.1.0, 2.1.0-preview1 Jan 12, 2018
@lakeman
Copy link
Author

lakeman commented Jan 25, 2018

So this seems to fix the problem;

                 var key = xEntry.EntityType.FindPrimaryKey();
+                var ykey = (xEntry.EntityType == yEntry.EntityType) ? key : yEntry.EntityType.FindPrimaryKey();

...

-                    result = compare(xEntry.GetCurrentValue(keyProperty), yEntry.GetCurrentValue(keyProperty));
+                    result = compare(xEntry.GetCurrentValue(keyProperty), yEntry.GetCurrentValue(ykey.Properties[i]));

Which I've tested by replacing the IComparer<ModificationCommand> service with a patched version in my reproducer example above.

@dasMulli
Copy link

dasMulli commented Feb 19, 2018

I'm running into this as well, after some new entities and relations were introduced. I'm also unable to exactly pinpoint the preconditions, but @lakeman's workaround fixes it (❤️ ).

Stack Trace
System.InvalidOperationException: Failed to compare two elements in the array. ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method(Closure , Object , Object )
   at Microsoft.EntityFrameworkCore.Update.Internal.ModificationCommandComparer.Compare(ModificationCommand x, ModificationCommand y)
   at System.Collections.Generic.ArraySortHelper`1.InsertionSort(T[] keys, Int32 lo, Int32 hi, Comparison`1 comparer)
   at System.Collections.Generic.ArraySortHelper`1.IntrospectiveSort(T[] keys, Int32 left, Int32 length, Comparison`1 comparer)
   at System.Collections.Generic.ArraySortHelper`1.Sort(T[] keys, Int32 index, Int32 length, IComparer`1 comparer)
   --- End of inner exception stack trace ---
   at System.Collections.Generic.ArraySortHelper`1.Sort(T[] keys, Int32 index, Int32 length, IComparer`1 comparer)
   at System.Collections.Generic.List`1.Sort(Int32 index, Int32 count, IComparer`1 comparer)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__8.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__10.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.<ExecuteAsync>d__7`2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__61.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__59.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Microsoft.EntityFrameworkCore.DbContext.<SaveChangesAsync>d__48.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Avs.DealerPortal.Services.OrganizationalUnits.OrganizationalUnitSynchronizer.<SynchronizeOrganizationalUnits>d__3.MoveNext() in C:\repos\avs-portal\src\Avs.DealerPortal\Services\OrganizationalUnits\OrganizationalUnitSynchronizer.cs:line 60
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Avs.DealerPortal.Services.OrganizationalUnits.OrganizationalUnitSynchronizationHostedService.<DoScheduledWorkAsync>d__7.MoveNext() in C:\repos\avs-portal\src\Avs.DealerPortal\Services\OrganizationalUnits\OrganizationalUnitSynchronizationHostedService.cs:line 64}
    Data: {System.Collections.ListDictionaryInternal}
    HResult: -2146233079
    HelpLink: null
    InnerException: {System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method(Closure , Object , Object )
   at Microsoft.EntityFrameworkCore.Update.Internal.ModificationCommandComparer.Compare(ModificationCommand x, ModificationCommand y)
   at System.Collections.Generic.ArraySortHelper`1.InsertionSort(T[] keys, Int32 lo, Int32 hi, Comparison`1 comparer)
   at System.Collections.Generic.ArraySortHelper`1.IntrospectiveSort(T[] keys, Int32 left, Int32 length, Comparison`1 comparer)
   at System.Collections.Generic.ArraySortHelper`1.Sort(T[] keys, Int32 index, Int32 length, IComparer`1 comparer)}
    Message: "Failed to compare two elements in the array."
    Source: "System.Private.CoreLib"
    StackTrace: "   at System.Collections.Generic.ArraySortHelper`1.Sort(T[] keys, Int32 index, Int32 length, IComparer`1 comparer)\r\n   at System.Collections.Generic.List`1.Sort(Int32 index, Int32 count, IComparer`1 comparer)\r\n   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__8.MoveNext()\r\n   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__10.MoveNext()\r\n--- End of stack trace from previous location where exception was thrown ---\r\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n   at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.<ExecuteAsync>d__7`2.MoveNext()\r\n--- End of stack trace from previous location where exception was thrown ---\r\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSucce
ssAndDebuggerNotification(Task task)\r\n   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()\r\n   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__61.MoveNext()\r\n--- End of stack trace from previous location where exception was thrown ---\r\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()\r\n   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__59.MoveNext()\r\n--- End of stack trace from previous location where exception was thrown ---\r\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()\r\n   at Microsoft.EntityFrameworkCore.DbContext.<Sav
eChangesAsync>d__48.MoveNext()\r\n--- End of stack trace from previous location where exception was thrown ---\r\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()\r\n   at Avs.DealerPortal.Services.OrganizationalUnits.OrganizationalUnitSynchronizer.<SynchronizeOrganizationalUnits>d__3.MoveNext() in C:\\repos\\avs-portal\\src\\Avs.DealerPortal\\Services\\OrganizationalUnits\\OrganizationalUnitSynchronizer.cs:line 60\r\n--- End of stack trace from previous location where exception was thrown ---\r\n   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()\r\n   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n   at Avs.DealerPortal.Services.OrganizationalUnits.OrganizationalUnitSynchronizationHostedService.<DoScheduledWorkAsync>d__7.MoveNext() in C:\\repos\\avs-p
ortal\\src\\Avs.DealerPortal\\Services\\OrganizationalUnits\\OrganizationalUnitSynchronizationHostedService.cs:line 64"
    TargetSite: {Void Sort(T[], Int32, Int32, System.Collections.Generic.IComparer`1[T])}

@ajcvickers could this be considered for a 2.0 patch?

@dasMulli
Copy link

I've been seeing another instance of this surfacing as InvalidCastException. After deploying @lakeman's fix to production, it is gone.

Stack Trace (first type is an owned type of the second one)
System.InvalidOperationException: Failed to compare two elements in the array.
 ---> System.InvalidCastException: Unable to cast object of type 'Avs.DealerPortal.Data.DbEntities.CustomVehicleInsuranceConfiguration' to type 'Avs.DealerPortal.Data.DbEntities.OrganizationalUnit'.
at Microsoft.EntityFrameworkCore.Metadata.Internal.ClrPropertyGetter`2.GetClrValue(System.Object instance)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.ReadPropertyValue(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase propertyBase)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalMixedEntityEntry.ReadPropertyValue(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase propertyBase) at offset 8
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.get_Item(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase propertyBase) at offset 24
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.GetCurrentValue(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase propertyBase) at offset 21
at Microsoft.EntityFrameworkCore.Update.Internal.ModificationCommandComparer.Compare(Microsoft.EntityFrameworkCore.Update.ModificationCommand x, Microsoft.EntityFrameworkCore.Update.ModificationCommand y) at offset 194
at System.Collections.Generic.ArraySortHelper`1.SwapIfGreater(System.Collections.Generic.T[] keys, System.Comparison`1 comparer, System.Int32 a, System.Int32 b) at offset 4
at System.Collections.Generic.ArraySortHelper`1.PickPivotAndPartition(System.Collections.Generic.T[] keys, System.Int32 lo, System.Int32 hi, System.Comparison`1 comparer) at offset 17
at System.Collections.Generic.ArraySortHelper`1.IntroSort(System.Collections.Generic.T[] keys, System.Int32 lo, System.Int32 hi, System.Int32 depthLimit, System.Comparison`1 comparer) at offset 105
at System.Collections.Generic.ArraySortHelper`1.IntrospectiveSort(System.Collections.Generic.T[] keys, System.Int32 left, System.Int32 length, System.Comparison`1 comparer) at offset 28
at System.Collections.Generic.ArraySortHelper`1.Sort(System.Collections.Generic.T[] keys, System.Int32 index, System.Int32 length, System.Collections.Generic.IComparer`1 comparer) at offset 28
--- End of inner exception stack trace ---
at System.Collections.Generic.ArraySortHelper`1.Sort(System.Collections.Generic.T[] keys, System.Int32 index, System.Int32 length, System.Collections.Generic.IComparer`1 comparer) at offset 57
at System.Collections.Generic.List`1.Sort(System.Int32 index, System.Int32 count, System.Collections.Generic.IComparer`1 comparer) at offset 57
at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__8.MoveNext() at offset 155
at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.<ExecuteAsync>d__10.MoveNext() at offset 537
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at offset 12
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task task) at offset 46
at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.<ExecuteAsync>d__7`2.MoveNext() at offset 180
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at offset 12
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task task) at offset 46
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at offset 11
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__61.MoveNext() at offset 136
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at offset 12
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task task) at offset 46
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at offset 11
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<SaveChangesAsync>d__59.MoveNext() at offset 250
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at offset 12
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task task) at offset 46
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at offset 11
at Microsoft.EntityFrameworkCore.DbContext.<SaveChangesAsync>d__48.MoveNext() at offset 136

@lakeman
Copy link
Author

lakeman commented Feb 19, 2018

The problem is related to sorting the pending changes so they can be batched together. Hitting the bug depends on the sort routine comparing two records with different combinations of types and owned types for different rows of the same table. The sort routine doesn't need to compare every pair of records to order the collection.

GetCurrentValue() has different implementations depending on how each type stores the same value. So it will throw different exception types for different combinations.

My reproducer above, uses a rather brute force method to fill the set of pending changes with different permutations of objects. Writing a test case for this fix probably requires passing an explicit pair of carefully constructed objects directly to ModificationCommandComparer.

@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 Mar 1, 2018
@AndriySvyryd AndriySvyryd removed their assignment Mar 1, 2018
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview2, 2.1.0 Nov 11, 2019
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

4 participants