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

Fix to #23779 - TryRewriteEntityEquality and NotEqual comparison of CompositePrimaryKey #23821

Merged
merged 3 commits into from
Jan 13, 2021

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Jan 5, 2021

When comparing composite key entities we were comparing the constituent keys and combining them with AndAlso. For not equal comparison we were still comparing the keys using !=, but we didn't flip the combining operator to OrElse.

Fixes #23779

@maumar maumar requested review from roji and smitpatel January 5, 2021 21:58
@dmitry-lipetsk
Copy link
Contributor

dmitry-lipetsk commented Jan 6, 2021

Could you look to this code also?

result = Visit(
primaryKeyProperties1.Select(
p =>
{
var comparison = Expression.Call(
_objectEqualsMethodInfo,
Expression.Convert(CreatePropertyAccessExpression(nonNullEntityReference, p), typeof(object)),
Expression.Convert(Expression.Constant(null, p.ClrType.MakeNullable()), typeof(object)));
return nodeType == ExpressionType.Equal
? (Expression)comparison
: Expression.Not(comparison);
}).Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r)));

It is very strange, that Equal case uses OrElse operator and NotEqual case uses AndAlso.


If this is rule for comparison with NULL (entity equal to NULL, when any part contains NULL), then this code is ok...

@maumar
Copy link
Contributor Author

maumar commented Jan 8, 2021

@dmitry-lipetsk wrt null comparison, this is correct. We consider null key when any of the parts has null value

…ompositePrimaryKey

When comparing composite key entities we were comparing the constituent keys and combining them with AndAlso. For not equal comparison we were still comparing the keys using !=, but we didn't flip the combining operator to OrElse.

Fixes #23779
…ompositePrimaryKey

When comparing composite key entities we were comparing the constituent keys and combining them with AndAlso. For not equal comparison we were still comparing the keys using !=, but we didn't flip the combining operator to OrElse.

Fixes #23779
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TryRewriteEntityEquality and NotEqual comparison of CompositePrimaryKey
3 participants