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 :: NRE protection is not applied to manually created GroupJoin when trying to extract keys from collections potentially containing nulls #6429

Closed
maumar opened this issue Aug 29, 2016 · 1 comment
Closed

Query :: NRE protection is not applied to manually created GroupJoin when trying to extract keys from collections potentially containing nulls #6429

maumar opened this issue Aug 29, 2016 · 1 comment
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
1.1.0

Comments

@maumar
Copy link
Contributor

maumar commented Aug 29, 2016

Example of a query that could throw NRE:

context.LevelOne
                    .GroupJoin(
                        context.LevelOne.Where(l1 => l1.Name != "L1 01").Select(l1 => l1.OneToOne_Required_FK),
                        l1 => l1.Id,
                        l2 => l2.Level1_Optional_Id,
                        (l1, l2s) => new { l1, l2s })
                    .Where(r => r.l2s.Any())
                    .Select(r => r.l1);

this throws if one (or more) inner element is null - we don't apply null protection logic to the key extraction, so we try to access Level1_Optional_Id of a null-valued Level2 item.

We already do there correct thing if GroupJoin is created by us (i.e. when we rewrite optional navigations), but we should do it for all cases. Current workaround is to simply add null protection logic yourself, in a form of l2 != null ? l2.Level1_Optional_Id : null
@divega divega added this to the 1.1.0 milestone Aug 31, 2016
@maumar maumar mentioned this issue Aug 31, 2016
Closed
maumar added a commit that referenced this issue Sep 21, 2016
@maumar
Fix to #6429 - NRE protection is not applied to manually created Grou…
7787ceb 
…pJoin when trying to extract keys from collections potentially containing nulls
maumar added a commit that referenced this issue Sep 26, 2016
@maumar
Fix to #6429 - NRE protection is not applied to manually created Grou…
608ae15 
…pJoin when trying to extract keys from collections potentially containing nulls

Fix to #6609 - Query :: we are adding redundant navigation joins to query in some cases, resulting in correct but unnecessarily complex queries

Problem was that for some groupjoin queries, e.g.:

var query = context.LevelOne
    .GroupJoin(
        context.LevelOne.Select(l1 => l1.OneToOne_Required_FK),
        l1 => l1.Id,
        l2 => EF.Property<int?>(l2, "Level1_Optional_Id"),
        (l1, l2s) => new { l1, l2s })
    .Select(r => r.l1)

In this case inner collection selector can potentially return null and therefore when translating inner key selector we should apply null protection logic. We were not doing that however, because those two operations were done in separate places, by two different instances of NavigationRewritingExpressionVisitor. Fix is to introduce additional state to the visitor, when visiting inner key selector, and then using that information when processing inner key selector.

Moreover now we are reusing the NavigationRewritingExpressionVisitor simplifying the code somewhat and allowing us to easily share state between different nav rewrite operations.

This also fixes #6609 since now we are better at recognizing the same navigation joins (as they are processed by the same visitor)
maumar added a commit that referenced this issue Sep 27, 2016
@maumar
Fix to #6429 - NRE protection is not applied to manually created Grou…
9897064 
…pJoin when trying to extract keys from collections potentially containing nulls

Fix to #6609 - Query :: we are adding redundant navigation joins to query in some cases, resulting in correct but unnecessarily complex queries

Problem was that for some groupjoin queries, e.g.:

var query = context.LevelOne
    .GroupJoin(
        context.LevelOne.Select(l1 => l1.OneToOne_Required_FK),
        l1 => l1.Id,
        l2 => EF.Property<int?>(l2, "Level1_Optional_Id"),
        (l1, l2s) => new { l1, l2s })
    .Select(r => r.l1)

In this case inner collection selector can potentially return null and therefore when translating inner key selector we should apply null protection logic. We were not doing that however, because those two operations were done in separate places, by two different instances of NavigationRewritingExpressionVisitor. Fix is to introduce additional state to the visitor, when visiting inner key selector, and then using that information when processing inner key selector.

Moreover now we are reusing the NavigationRewritingExpressionVisitor simplifying the code somewhat and allowing us to easily share state between different nav rewrite operations.

This also fixes #6609 since now we are better at recognizing the same navigation joins (as they are processed by the same visitor)
@maumar
Copy link
Contributor Author

maumar commented Sep 27, 2016

fixed in 9897064

@maumar maumar closed this as completed Sep 27, 2016
@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 27, 2016
@ajcvickers ajcvickers modified the milestones: 1.1.0-preview1, 1.1.0 Oct 15, 2022
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
1.1.0
Development

No branches or pull requests
3 participants
@divega @ajcvickers @maumar