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.

Sign up for GitHub

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

Jump to bottom

Query: incorrect results for queries with optional navigation followed by collection navigation with skip/take/distinct #17531

Closed
maumar opened this issue Aug 30, 2019 · 2 comments · Fixed by #17669
Closed

Query: incorrect results for queries with optional navigation followed by collection navigation with skip/take/distinct #17531

maumar opened this issue Aug 30, 2019 · 2 comments · Fixed by #17669
Assignees
@maumar
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone
3.1.0

Comments

@maumar
Copy link
Contributor

maumar commented Aug 30, 2019 •
edited
Loading

query:

from l1 in ctx.LevelOnes
select l1.OneToOne_Optional_FK1.OneToMany_Optional2.Take(50)

generated sql:

SELECT l.Id, t.Id, t.Level2_Optional_Id, t.Level2_Required_Id, t.Name, t.OneToMany_Optional_Inverse3Id, t.OneToMany_Optional_Self_Inverse3Id, t.OneToMany_Required_Inverse3Id, t.OneToMany_Required_Self_Inverse3Id, t.OneToOne_Optional_PK_Inverse3Id, t.OneToOne_Optional_Self3Id
    FROM LevelOne AS l
    LEFT JOIN LevelTwo AS l1 ON l.Id == l1.Level1_Optional_Id
    OUTER APPLY 
    (
        SELECT TOP(50) l0.Id, l0.Level2_Optional_Id, l0.Level2_Required_Id, l0.Name, l0.OneToMany_Optional_Inverse3Id, l0.OneToMany_Optional_Self_Inverse3Id, l0.OneToMany_Required_Inverse3Id, l0.OneToMany_Required_Self_Inverse3Id, l0.OneToOne_Optional_PK_Inverse3Id, l0.OneToOne_Optional_Self3Id
        FROM LevelThree AS l0
        WHERE ((l1.Id == l0.OneToMany_Optional_Inverse3Id) && ( l1.Id IS NOT NULL && l0.OneToMany_Optional_Inverse3Id IS NOT NULL)) || (l1.Id IS NULL &&  l0.OneToMany_Optional_Inverse3Id IS NULL)
    ) AS t
    ORDER BY l.Id ASC, t.Id ASC

Problem is that initially in nav rewrite for collection navigation we introduce correlation predicate in form of equality. In simple case (i.e. without Skip/Take/Distinct) that gets converted into join, and the correlation predicate gets converted to join condition.

However here, we can't do that, so the collection remains as a subquery and the correlation predicate remains equality. Then, null semantics gets applied and if both sides of the correlation predicate are null, the result is a match - which is incorrect in case of correlating navigations.

We should add condition to the correlation predicate that would filter out cases where outer key is null. We should do it in nav rewrite and then compensate accordingly in relational for the new shape of the correlation predicate
@maumar
Copy link
Contributor Author

maumar commented Aug 30, 2019

related to #15798

@ajcvickers ajcvickers mentioned this issue Aug 30, 2019
Closed
25 tasks
@smitpatel smitpatel changed the title Query: incorrect results for queries with optional navigation followed by collection navigation with skip/take/distinct InMemory: Collection navigation matches null == null Aug 31, 2019
@maumar
Copy link
Contributor Author

maumar commented Sep 3, 2019

@smitpatel this is not exclusive to InMemory, previous title was correct

@maumar maumar changed the title InMemory: Collection navigation matches null == null Query: incorrect results for queries with optional navigation followed by collection navigation with skip/take/distinct Sep 3, 2019
@maumar maumar self-assigned this Sep 3, 2019
@ajcvickers ajcvickers added this to the 3.1.0 milestone Sep 6, 2019
maumar added a commit that referenced this issue Sep 6, 2019
@maumar
Fix to #17531 - Query: incorrect results for queries with optional na…
6da0544 
…vigation followed by collection navigation with skip/take/distinct

Problem was that when expanding collection navigations we convert them into subqueries with a correlation predicate being outerKey == innerKey. For relational, most of those queries would later be converted into joins, however for some complex cases e.g. with Skip/Take (and also on InMemory) the query would stay in the form of subquery with correlation predicate. Then, null semantics kicks in and converts the correlation predicate to a form that returns true when both keys are null. This is incorrect in the context of chaining navigations - if the parent entity is null then it should never return any children.

Fix is to add null check to the correlation predicate during nav rewrite, like so: outerKey != null && outerKey == innerKey
Additionally, when trying to convert those subqueries into joins we need to account for a new pattern and remove the null check, since its irrelevant when it comes to join key comparison on relational
@maumar maumar mentioned this issue Sep 6, 2019
Merged
maumar added a commit that referenced this issue Sep 9, 2019
@maumar
Fix to #17531 - Query: incorrect results for queries with optional na…
12174a1 
…vigation followed by collection navigation with skip/take/distinct

Problem was that when expanding collection navigations we convert them into subqueries with a correlation predicate being outerKey == innerKey. For relational, most of those queries would later be converted into joins, however for some complex cases e.g. with Skip/Take (and also on InMemory) the query would stay in the form of subquery with correlation predicate. Then, null semantics kicks in and converts the correlation predicate to a form that returns true when both keys are null. This is incorrect in the context of chaining navigations - if the parent entity is null then it should never return any children.

Fix is to add null check to the correlation predicate during nav rewrite, like so: outerKey != null && outerKey == innerKey
Additionally, when trying to convert those subqueries into joins we need to account for a new pattern and remove the null check, since its irrelevant when it comes to join key comparison on relational
maumar added a commit that referenced this issue Sep 10, 2019
@maumar
Fix to #17531 - Query: incorrect results for queries with optional na…
90fc979 
…vigation followed by collection navigation with skip/take/distinct

Problem was that when expanding collection navigations we convert them into subqueries with a correlation predicate being outerKey == innerKey. For relational, most of those queries would later be converted into joins, however for some complex cases e.g. with Skip/Take (and also on InMemory) the query would stay in the form of subquery with correlation predicate. Then, null semantics kicks in and converts the correlation predicate to a form that returns true when both keys are null. This is incorrect in the context of chaining navigations - if the parent entity is null then it should never return any children.

Fix is to add null check to the correlation predicate during nav rewrite, like so: outerKey != null && outerKey == innerKey
Additionally, when trying to convert those subqueries into joins we need to account for a new pattern and remove the null check, since its irrelevant when it comes to join key comparison on relational
maumar added a commit that referenced this issue Sep 10, 2019
@maumar
Fix to #17531 - Query: incorrect results for queries with optional na…
5386e00 
…vigation followed by collection navigation with skip/take/distinct

Problem was that when expanding collection navigations we convert them into subqueries with a correlation predicate being outerKey == innerKey. For relational, most of those queries would later be converted into joins, however for some complex cases e.g. with Skip/Take (and also on InMemory) the query would stay in the form of subquery with correlation predicate. Then, null semantics kicks in and converts the correlation predicate to a form that returns true when both keys are null. This is incorrect in the context of chaining navigations - if the parent entity is null then it should never return any children.

Fix is to add null check to the correlation predicate during nav rewrite, like so: outerKey != null && outerKey == innerKey
Additionally, when trying to convert those subqueries into joins we need to account for a new pattern and remove the null check, since its irrelevant when it comes to join key comparison on relational
maumar added a commit that referenced this issue Sep 10, 2019
@maumar
Fix to #17531 - Query: incorrect results for queries with optional na…
2053e62 
…vigation followed by collection navigation with skip/take/distinct

Problem was that when expanding collection navigations we convert them into subqueries with a correlation predicate being outerKey == innerKey. For relational, most of those queries would later be converted into joins, however for some complex cases e.g. with Skip/Take (and also on InMemory) the query would stay in the form of subquery with correlation predicate. Then, null semantics kicks in and converts the correlation predicate to a form that returns true when both keys are null. This is incorrect in the context of chaining navigations - if the parent entity is null then it should never return any children.

Fix is to add null check to the correlation predicate during nav rewrite, like so: outerKey != null && outerKey == innerKey
Additionally, when trying to convert those subqueries into joins we need to account for a new pattern and remove the null check, since its irrelevant when it comes to join key comparison on relational
@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 Sep 10, 2019
@smitpatel smitpatel mentioned this issue Sep 27, 2019
Closed
@ajcvickers ajcvickers modified the milestones: 3.1.0, 3.1.0-preview1 Oct 15, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0-preview1, 3.1.0 Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maumar maumar

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
Milestone
3.1.0
3 participants
@ajcvickers @smitpatel @maumar