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 :: we are adding redundant navigation joins to query in some cases, resulting in correct but unnecessarily complex queries #6609

Closed
maumar opened this issue Sep 26, 2016 · 0 comments
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 Sep 26, 2016

using GearsOfWar model:

                var tags = new List<CogTag>();

                var gears = context.Gears
                    .Where(g => g.Tag != null && tags.Contains(g.Tag.Id))
                    .ToList();

produces the following query model (after navigation expansion)

from Gear g in Gears 
join CogTag g.Tag in CogTags
on 
new CompositeKey(new [] {Convert(Property([g], "Nickname")), Convert(Property([g], "SquadId"))}) 
equals 
new CompositeKey(new [] {Convert(Property([g.Tag], "GearNickName")), Convert(Property([g.Tag], "GearSquadId"))}) 
into IEnumerable`1 g.Tag_group 
from CogTag g.Tag in {[g.Tag_group] => DefaultIfEmpty()} 
join CogTag g.Tag in CogTags) 
on new CompositeKey(new [] {Convert(Property([g], "Nickname")), Convert(Property([g], "SquadId"))}) 
equals 
new CompositeKey(new [] {Convert(Property([g.Tag], "GearNickName")), Convert(Property([g.Tag], "GearSquadId"))}) 
into IEnumerable`1 g.Tag_group 
from CogTag g.Tag in {[g.Tag_group] => DefaultIfEmpty()} 
where (([g.Tag] != null) AndAlso {__tags_0 => Contains([g?.Tag]?.Id)}) 
select [g]

Basically we join CogTag twice even though it's the same reference. Problem is that second time CogTag is used inside a subquery and we are not smart enough to associate those two together. This causes additional join and therefore (currently) additional query issued to the database:

SELECT [g.Tag1].[Id], [g.Tag1].[GearNickName], [g.Tag1].[GearSquadId], [g.Tag1].[Note]
FROM [CogTag] AS [g.Tag1]

SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOrBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], [g.Tag].[Id], [g.Tag].[GearNickName], [g.Tag].[GearSquadId], [g.Tag].[Note]
FROM [Gear] AS [g]
LEFT JOIN [CogTag] AS [g.Tag] ON ([g].[Nickname] = [g.Tag].[GearNickName]) AND ([g].[SquadId] = [g.Tag].[GearSquadId])
WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND [g.Tag].[Id] IS NOT NULL
ORDER BY [g].[Nickname], [g].[SquadId]
@maumar maumar self-assigned this Sep 26, 2016
maumar added a commit that referenced this issue Sep 26, 2016
…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
…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 maumar added this to the 1.1.0 milestone Sep 27, 2016
@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 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
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

2 participants