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

Query: comparing collection navigations to null or to each other doesn't work #8364

Closed
maumar opened this issue May 3, 2017 · 2 comments
Closed
Assignees
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

@maumar
Copy link
Contributor

maumar commented May 3, 2017

We support comparisons of qsres/reference navigations to nulls and to each other via EntityEqualityRewritingExpressionVisitor, however collection navigations are not handled and cause exceptions later in the query compilation phase.

We can recognize a few patterns to make the query models much easier for us to translate:

1.) c.Customers == null --> c == null --> c.Id == null
2.) c1.Customers == c2.Customers --> c1 == c2 --> c1.Id == c2.Id

For both cases we should issue a warning that this may not be intended behavior (e.g. c.Orders== null, could be intended to check if the customer has any orders)

@maumar
Copy link
Contributor Author

maumar commented May 3, 2017

related to #5825

maumar added a commit that referenced this issue May 3, 2017
…each other doesn't work

Also fixes (smaller in scope) #5825 - Query: Comparing collection navigation to NULL throws Null Exception

Problem was that we were not taking into account queries where collection navigation was being compared to null and/or each other. This would cause various exceptions during query compilation.

Some of the common cases can be simplified, making them much easier for the query compiler (i.e. navigation rewrite) to process.

When it comes to null comparison, we assume that collection navigation is only null if its parent entity is null, so we can actually just compare the parent entity to null instead
e.g.: customer.Orders == null ---> customer == null ---> customer.Id == null

However, it's possible that user wanted to check whether collection has any elements - we will issue a warning that the comparison to null could have unintended consequences and suggest to use .Any() instead.

When collection navigations are compared to each other we can also approximate by comparing the parent entities instead (if the collections are the same INavigation). If navigations are different, we assume they are never equal.
e.g.:
csutomer1.Orders == customer2.Orders ---> customer1.Id == customer2.Id
customer.Orders == company.ShippedOrders ---> false

This is not a perfect translation, since there can be two collections with exactly the same elements coming from different navigations (e.g. two empty collections: c.CompletedOrders, c.PendingOrders)
Therefore we also issue the warning in this case, saying that the result may not be what user expected.
maumar added a commit that referenced this issue May 3, 2017
…each other doesn't work

Also fixes (smaller in scope) #5825 - Query: Comparing collection navigation to NULL throws Null Exception
And #8366 - Query: EntityEqualityRewritingExpressionVisitor doesn't work with object.Equals method

Problem was that we were not taking into account queries where collection navigation was being compared to null and/or each other. This would cause various exceptions during query compilation.

Some of the common cases can be simplified, making them much easier for the query compiler (i.e. navigation rewrite) to process.

When it comes to null comparison, we assume that collection navigation is only null if its parent entity is null, so we can actually just compare the parent entity to null instead
e.g.: customer.Orders == null ---> customer == null ---> customer.Id == null

However, it's possible that user wanted to check whether collection has any elements - we will issue a warning that the comparison to null could have unintended consequences and suggest to use .Any() instead.

When collection navigations are compared to each other we can also approximate by comparing the parent entities instead (if the collections are the same INavigation). If navigations are different, we assume they are never equal.
e.g.:
csutomer1.Orders == customer2.Orders ---> customer1.Id == customer2.Id
customer.Orders == company.ShippedOrders ---> false

This is not a perfect translation, since there can be two collections with exactly the same elements coming from different navigations (e.g. two empty collections: c.CompletedOrders, c.PendingOrders)
Therefore we also issue the warning in this case, saying that the result may not be what user expected.

When Equals method is used (both static and instance version) we simply translate it to a == b and revisit with existing logic.
maumar added a commit that referenced this issue May 4, 2017
…each other doesn't work

Also fixes (smaller in scope) #5825 - Query: Comparing collection navigation to NULL throws Null Exception
And #8366 - Query: EntityEqualityRewritingExpressionVisitor doesn't work with object.Equals method

Problem was that we were not taking into account queries where collection navigation was being compared to null and/or each other. This would cause various exceptions during query compilation.

Some of the common cases can be simplified, making them much easier for the query compiler (i.e. navigation rewrite) to process.

When it comes to null comparison, we assume that collection navigation is only null if its parent entity is null, so we can actually just compare the parent entity to null instead
e.g.: customer.Orders == null ---> customer == null ---> customer.Id == null

However, it's possible that user wanted to check whether collection has any elements - we will issue a warning that the comparison to null could have unintended consequences and suggest to use .Any() instead.

When collection navigations are compared to each other we can also approximate by comparing the parent entities instead (if the collections are the same INavigation). If navigations are different, we assume they are never equal.
e.g.:
csutomer1.Orders == customer2.Orders ---> customer1.Id == customer2.Id
customer.Orders == company.ShippedOrders ---> false

This is not a perfect translation, since there can be two collections with exactly the same elements coming from different navigations (e.g. two empty collections: c.CompletedOrders, c.PendingOrders)
Therefore we also issue the warning in this case, saying that the result may not be what user expected.

When Equals method is used (both static and instance version) we simply translate it to a == b and revisit with existing logic.
maumar added a commit that referenced this issue May 4, 2017
…each other doesn't work

Also fixes (smaller in scope) #5825 - Query: Comparing collection navigation to NULL throws Null Exception
And #8366 - Query: EntityEqualityRewritingExpressionVisitor doesn't work with object.Equals method

Problem was that we were not taking into account queries where collection navigation was being compared to null and/or each other. This would cause various exceptions during query compilation.

Some of the common cases can be simplified, making them much easier for the query compiler (i.e. navigation rewrite) to process.

When it comes to null comparison, we assume that collection navigation is only null if its parent entity is null, so we can actually just compare the parent entity to null instead
e.g.: customer.Orders == null ---> customer == null ---> customer.Id == null

However, it's possible that user wanted to check whether collection has any elements - we will issue a warning that the comparison to null could have unintended consequences and suggest to use .Any() instead.

When collection navigations are compared to each other we can also approximate by comparing the parent entities instead (if the collections are the same INavigation). If navigations are different, we assume they are never equal.
e.g.:
csutomer1.Orders == customer2.Orders ---> customer1.Id == customer2.Id
customer.Orders == company.ShippedOrders ---> false

This is not a perfect translation, since there can be two collections with exactly the same elements coming from different navigations (e.g. two empty collections: c.CompletedOrders, c.PendingOrders)
Therefore we also issue the warning in this case, saying that the result may not be what user expected.

When Equals method is used (both static and instance version) we simply translate it to a == b and revisit with existing logic.
maumar added a commit that referenced this issue May 4, 2017
…each other doesn't work

Also fixes (smaller in scope) #5825 - Query: Comparing collection navigation to NULL throws Null Exception
And #8366 - Query: EntityEqualityRewritingExpressionVisitor doesn't work with object.Equals method

Problem was that we were not taking into account queries where collection navigation was being compared to null and/or each other. This would cause various exceptions during query compilation.

Some of the common cases can be simplified, making them much easier for the query compiler (i.e. navigation rewrite) to process.

When it comes to null comparison, we assume that collection navigation is only null if its parent entity is null, so we can actually just compare the parent entity to null instead
e.g.: customer.Orders == null ---> customer == null ---> customer.Id == null

However, it's possible that user wanted to check whether collection has any elements - we will issue a warning that the comparison to null could have unintended consequences and suggest to use .Any() instead.

When collection navigations are compared to each other we can also approximate by comparing the parent entities instead (if the collections are the same INavigation). If navigations are different, we assume they are never equal.
e.g.:
csutomer1.Orders == customer2.Orders ---> customer1.Id == customer2.Id
customer.Orders == company.ShippedOrders ---> false

This is not a perfect translation, since there can be two collections with exactly the same elements coming from different navigations (e.g. two empty collections: c.CompletedOrders, c.PendingOrders)
Therefore we also issue the warning in this case, saying that the result may not be what user expected.

When Equals method is used (both static and instance version) we simply translate it to a == b and revisit with existing logic.
@maumar
Copy link
Contributor Author

maumar commented May 4, 2017

fixed in c4129dc

@maumar maumar closed this as completed May 4, 2017
@maumar maumar self-assigned this May 4, 2017
@maumar maumar added type-bug closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels May 4, 2017
@maumar maumar added this to the 2.0.0 milestone May 4, 2017
@bricelam bricelam modified the milestones: 2.0.0, 2.0.0-preview2 May 16, 2017
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview2, 2.0.0 Oct 15, 2022
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

3 participants