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

TryRewriteEntityEquality and NotEqual comparison of CompositePrimaryKey #23779

Closed
dmitry-lipetsk opened this issue Dec 29, 2020 · 1 comment · Fixed by #23821
Closed

TryRewriteEntityEquality and NotEqual comparison of CompositePrimaryKey #23779

dmitry-lipetsk opened this issue Dec 29, 2020 · 1 comment · Fixed by #23821
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@dmitry-lipetsk
Copy link
Contributor

I not sure, but seems this code has mistake:

result = Visit(
primaryKeyProperties.Select(
p =>
{
var comparison = Expression.Call(
_objectEqualsMethodInfo,
Expression.Convert(CreatePropertyAccessExpression(left, p), typeof(object)),
Expression.Convert(CreatePropertyAccessExpression(right, p), typeof(object)));
return nodeType == ExpressionType.Equal
? (Expression)comparison
: Expression.Not(comparison);
}).Aggregate((l, r) => Expression.AndAlso(l, r)));

In case of obj1.CompositePK != obj2.CompositePK need generate SQL with OR:

NOT(obj1.CompositePK.part1==obj2.CompositePK.part1) OR NOT(obj1.CompositePK.part2==obj2.CompositePK.part2)

Your code generates SQL with AND:

NOT(obj1.CompositePK.part1==obj2.CompositePK.part1) AND NOT(obj1.CompositePK.part2==obj2.CompositePK.part2)

?

@maumar
Copy link
Contributor

maumar commented Jan 5, 2021

it indeed is a bug which results in data corruption - possible candidate for patch? @ajcvickers

@maumar maumar removed this from the 6.0.0 milestone Jan 5, 2021
maumar added a commit that referenced this issue Jan 5, 2021
…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
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 5, 2021
maumar added a commit that referenced this issue Jan 5, 2021
…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
@AndriySvyryd AndriySvyryd added this to the 5.0.3 milestone Jan 8, 2021
maumar added a commit that referenced this issue Jan 9, 2021
…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
maumar added a commit that referenced this issue Jan 13, 2021
…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
@maumar maumar closed this as completed in 84227cd Jan 13, 2021
This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
4 participants