-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Allow FK type in database to be different from PK type #29026
Conversation
7dd25c8
to
19eb728
Compare
|
||
/// <summary> | ||
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to | ||
/// the same compatibility standards as public APIs. It may be changed or removed without notice in | ||
/// any release. You should only use it directly in your code with extreme caution and knowing that | ||
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public virtual IEqualityComparer<object?[]> EqualityComparer { get; } | ||
public virtual IEqualityComparer<object?[]> EqualityComparer { get; set; } = null!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public virtual IEqualityComparer<object?[]> EqualityComparer { get; set; } = null!; | |
public virtual IEqualityComparer<object?[]> EqualityComparer { get; protected set; } = null!; |
Good candidate to use required
IReadOnlyModificationCommand command, | ||
bool fromOriginalValues, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding work for the next cleanup, I see 🤔
} | ||
catch (TargetInvocationException exception) | ||
{ | ||
throw exception.InnerException ?? exception; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the users hit this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but is is rare; see tests. This would happen when using SaveChanges with store types for which we can't convert values. So, for example, and int and a GUID. But its hard to imagine a sane use of relationships, even without constraints in the database, that would get here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not worth it to add the try just to make the exception a bit better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, will remove.
|
||
using System.Transactions; | ||
|
||
namespace Microsoft.EntityFrameworkCore; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move it under .Update
Approach uses built-in value converters to convert FK value to type of PK. Fixes #28392
19eb728
to
533f6b4
Compare
Approach uses built-in value converters to convert FK value to type of PK.
Fixes #28392