Skip to content

Commit

Permalink
Fix to #6177 - Query: INNER JOIN generated for navigation traversals …
Browse files Browse the repository at this point in the history 
…from principal to dependents

Problem was that during navigation expansion we assumed that when Foreign Key is required, then automatically INNER JOIN could be produced to associate entities within the navigation. This is only true if the navigation is from dependent to principal.
If navigation is from principal to dependent need to produce LEFT OUTER JOIN as it is valid to have a principal entity without dependent in that case.
  • Loading branch information
@maumar
maumar committed Aug 1, 2016
1 parent a8c05dd commit 21dc58c
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 73 deletions.
44 changes: 33 additions & 11 deletions src/Microsoft.EntityFrameworkCore.Specification.Tests/ComplexNavigationsQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public virtual void Key_equality_using_property_method_required()

var result = query.ToList();

Assert.Equal(3, result.Count);
Assert.Equal(4, result.Count);
}
}

Expand Down Expand Up @@ -164,9 +164,13 @@ public virtual void Multi_level_include_one_to_many_optional_and_one_to_many_opt
{
using (var context = CreateContext())
{
var expectedCount = context.LevelOne.Count();

ClearLog();

var result = context.LevelOne.Include(e => e.OneToMany_Optional).ThenInclude(e => e.OneToMany_Optional).ToList();

Assert.Equal(10, result.Count);
Assert.Equal(expectedCount, result.Count);

var level1 = result.Single(e => e.Name == "L1 01");

Expand All @@ -190,9 +194,13 @@ public virtual void Multi_level_include_correct_PK_is_chosen_as_the_join_predica
{
using (var context = CreateContext())
{
var expectedCount = context.LevelOne.Count();

ClearLog();

var result = context.LevelOne.Include(e => e.OneToMany_Optional).ThenInclude(e => e.OneToMany_Optional).ThenInclude(e => e.OneToMany_Required_Inverse.OneToMany_Optional).ToList();

Assert.Equal(10, result.Count);
Assert.Equal(expectedCount, result.Count);

var level1 = result.Single(e => e.Name == "L1 01");

Expand Down Expand Up @@ -466,10 +474,10 @@ where l2.OneToOne_Required_PK_Inverse?.Id > 5
}
}

[ConditionalFact]
// issue #5613
////[ConditionalFact]
public virtual void Navigation_inside_method_call_translated_to_join()
{

using (var context = CreateContext())
{
var query = from e1 in context.LevelOne
Expand All @@ -480,6 +488,20 @@ where e1.OneToOne_Required_FK.Name.StartsWith("L")
}
}

// issue #5613
////[ConditionalFact]
public virtual void Optional_navigation_inside_method_call_translated_to_join()
{
using (var context = CreateContext())
{
var query = from e1 in context.LevelOne
where e1.OneToOne_Optional_FK.Name.StartsWith("L")
select e1;

var result = query.ToList();
}
}

[ConditionalFact]
public virtual void Join_navigation_in_outer_selector_translated_to_extra_join()
{
Expand Down Expand Up @@ -704,7 +726,7 @@ join e1 in context.LevelOne on e3.Id equals e1.OneToOne_Required_FK.OneToOne_Opt
var result = query.ToList();

var expected = (from e3 in levelThrees
join e1 in levelOnes on e3.Id equals e1?.OneToOne_Required_FK.OneToOne_Optional_FK?.Id
join e1 in levelOnes on e3.Id equals e1?.OneToOne_Required_FK?.OneToOne_Optional_FK?.Id
select new { Id3 = e3.Id, Id1 = e1.Id }).ToList();

Assert.Equal(expected.Count, result.Count);
Expand Down Expand Up @@ -737,7 +759,7 @@ join e1 in context.LevelOne on e4.Name equals e1.OneToOne_Required_FK.OneToOne_O
var result = query.ToList();

var expected = (from e4 in levelFours
join e1 in levelOnes on e4.Name equals e1?.OneToOne_Required_FK.OneToOne_Optional_FK?.OneToOne_Required_PK?.Name
join e1 in levelOnes on e4.Name equals e1?.OneToOne_Required_FK?.OneToOne_Optional_FK?.OneToOne_Required_PK?.Name
select new { Id4 = e4.Id, Name4 = e4.Name, Id1 = e1.Id, Name1 = e1.Name }).ToList();

Assert.Equal(expected.Count, result.Count);
Expand Down Expand Up @@ -1452,7 +1474,7 @@ public virtual void Select_multiple_nav_prop_reference_required()

using (var context = CreateContext())
{
var query = context.LevelOne.Select(e => e.OneToOne_Required_FK.OneToOne_Required_FK.Id);
var query = context.LevelOne.Select(e => (int?)e.OneToOne_Required_FK.OneToOne_Required_FK.Id);
var result = query.ToList();

Assert.Equal(expected.Count, result.Count);
Expand Down Expand Up @@ -1734,7 +1756,7 @@ public virtual void Complex_navigations_with_predicate_projected_into_anonymous_
.Include(e => e.OneToOne_Required_FK.OneToOne_Optional_FK)
.ToList()
.Where(e =>
e.OneToOne_Required_FK.OneToOne_Required_FK.Id == e.OneToOne_Required_FK.OneToOne_Optional_FK?.Id
e.OneToOne_Required_FK?.OneToOne_Required_FK?.Id == e.OneToOne_Required_FK?.OneToOne_Optional_FK?.Id
&& e.OneToOne_Required_FK?.OneToOne_Optional_FK?.Id != 7)
.Select(e => new KeyValuePair<string, int?>
(
Expand Down Expand Up @@ -2265,7 +2287,7 @@ public virtual void SelectMany_nested_navigation_property_required()
expected = context.LevelOne
.Include(e => e.OneToOne_Required_FK.OneToMany_Optional)
.ToList()
.SelectMany(e => e.OneToOne_Required_FK.OneToMany_Optional).Select(e => e.Id)
.SelectMany(e => e.OneToOne_Required_FK?.OneToMany_Optional ?? new List<Level3>()).Select(e => e.Id)
.ToList();
}

Expand Down Expand Up @@ -2385,7 +2407,7 @@ public virtual void Where_navigation_property_to_collection()
.Include(l1 => l1.OneToOne_Required_FK)
.ThenInclude(l1 => l1.OneToMany_Optional)
.ToList()
.Where(l1 => l1?.OneToOne_Required_FK.OneToMany_Optional?.Count > 0)
.Where(l1 => l1?.OneToOne_Required_FK?.OneToMany_Optional?.Count > 0)
.Select(e => e?.Name)
.ToList();
}
Expand Down
22 changes: 16 additions & 6 deletions ...cification.Tests/TestModels/ComplexNavigationsModel/ComplexNavigationsModelInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ public static void Seed(ComplexNavigationsContext context)
var l1_08 = new Level1 { Id = 8, Name = "L1 08" };
var l1_09 = new Level1 { Id = 9, Name = "L1 09" };
var l1_10 = new Level1 { Id = 10, Name = "L1 10" };
var l1_11 = new Level1 { Id = 11, Name = "L1 11" };
var l1_12 = new Level1 { Id = 12, Name = "L1 12" };
var l1_13 = new Level1 { Id = 13, Name = "L1 13" };

var l2_01 = new Level2 { Id = 1, Name = "L2 01" };
var l2_02 = new Level2 { Id = 2, Name = "L2 02" };
Expand All @@ -31,6 +34,7 @@ public static void Seed(ComplexNavigationsContext context)
var l2_08 = new Level2 { Id = 8, Name = "L2 08" };
var l2_09 = new Level2 { Id = 9, Name = "L2 09" };
var l2_10 = new Level2 { Id = 10, Name = "L2 10" };
var l2_11 = new Level2 { Id = 11, Name = "L2 11" };

var l3_01 = new Level3 { Id = 1, Name = "L3 01" };
var l3_02 = new Level3 { Id = 2, Name = "L3 02" };
Expand All @@ -54,8 +58,8 @@ public static void Seed(ComplexNavigationsContext context)
var l4_09 = new Level4 { Id = 9, Name = "L4 09" };
var l4_10 = new Level4 { Id = 10, Name = "L4 10" };

var l1s = new[] { l1_01, l1_02, l1_03, l1_04, l1_05, l1_06, l1_07, l1_08, l1_09, l1_10 };
var l2s = new[] { l2_01, l2_02, l2_03, l2_04, l2_05, l2_06, l2_07, l2_08, l2_09, l2_10 };
var l1s = new[] { l1_01, l1_02, l1_03, l1_04, l1_05, l1_06, l1_07, l1_08, l1_09, l1_10, l1_11, l1_12, l1_13 };
var l2s = new[] { l2_01, l2_02, l2_03, l2_04, l2_05, l2_06, l2_07, l2_08, l2_09, l2_10, l2_11 };
var l3s = new[] { l3_01, l3_02, l3_03, l3_04, l3_05, l3_06, l3_07, l3_08, l3_09, l3_10 };
var l4s = new[] { l4_01, l4_02, l4_03, l4_04, l4_05, l4_06, l4_07, l4_08, l4_09, l4_10 };

Expand All @@ -74,6 +78,7 @@ public static void Seed(ComplexNavigationsContext context)
l1s[7].OneToOne_Required_PK = l2s[7];
l1s[8].OneToOne_Required_PK = l2s[8];
l1s[9].OneToOne_Required_PK = l2s[9];
l1s[10].OneToOne_Required_PK = l2s[10];

l1s[0].OneToOne_Required_FK = l2s[9];
l1s[1].OneToOne_Required_FK = l2s[8];
Expand All @@ -85,11 +90,12 @@ public static void Seed(ComplexNavigationsContext context)
l1s[7].OneToOne_Required_FK = l2s[2];
l1s[8].OneToOne_Required_FK = l2s[1];
l1s[9].OneToOne_Required_FK = l2s[0];
l1s[10].OneToOne_Required_FK = l2s[10];

l1s[0].OneToMany_Required = new List<Level2> { l2s[0], l2s[1], l2s[2], l2s[3], l2s[4], l2s[5], l2s[6], l2s[7], l2s[8], l2s[9] };
l1s[0].OneToMany_Required = new List<Level2> { l2s[0], l2s[1], l2s[2], l2s[3], l2s[4], l2s[5], l2s[6], l2s[7], l2s[8], l2s[9], l2s[10] };

l1s[0].OneToMany_Required_Self = new List<Level1> { l1s[0], l1s[1] };
l1s[1].OneToMany_Required_Self = new List<Level1> { l1s[2] };
l1s[0].OneToMany_Required_Self = new List<Level1> { l1s[0], l1s[1], l1s[11] };
l1s[1].OneToMany_Required_Self = new List<Level1> { l1s[2], l1s[12] };
l1s[2].OneToMany_Required_Self = new List<Level1> { l1s[3] };
l1s[3].OneToMany_Required_Self = new List<Level1> { l1s[4] };
l1s[4].OneToMany_Required_Self = new List<Level1> { l1s[5] };
Expand All @@ -98,6 +104,9 @@ public static void Seed(ComplexNavigationsContext context)
l1s[7].OneToMany_Required_Self = new List<Level1> { l1s[8] };
l1s[8].OneToMany_Required_Self = new List<Level1> { l1s[9] };
l1s[9].OneToMany_Required_Self = new List<Level1>();
l1s[10].OneToMany_Required_Self = new List<Level1>() { l1s[10] };
l1s[11].OneToMany_Required_Self = new List<Level1>();
l1s[12].OneToMany_Required_Self = new List<Level1>();

l2s[0].OneToOne_Required_PK = l3s[0];
l2s[1].OneToOne_Required_PK = l3s[1];
Expand All @@ -123,7 +132,7 @@ public static void Seed(ComplexNavigationsContext context)

l2s[0].OneToMany_Required = new List<Level3> { l3s[0], l3s[1], l3s[2], l3s[3], l3s[4], l3s[5], l3s[6], l3s[7], l3s[8], l3s[9] };

l2s[0].OneToMany_Required_Self = new List<Level2> { l2s[0], l2s[1] };
l2s[0].OneToMany_Required_Self = new List<Level2> { l2s[0], l2s[1], l2s[10] };
l2s[1].OneToMany_Required_Self = new List<Level2> { l2s[2] };
l2s[2].OneToMany_Required_Self = new List<Level2> { l2s[3] };
l2s[3].OneToMany_Required_Self = new List<Level2> { l2s[4] };
Expand All @@ -133,6 +142,7 @@ public static void Seed(ComplexNavigationsContext context)
l2s[7].OneToMany_Required_Self = new List<Level2> { l2s[8] };
l2s[8].OneToMany_Required_Self = new List<Level2> { l2s[9] };
l2s[9].OneToMany_Required_Self = new List<Level2>();
l2s[10].OneToMany_Required_Self = new List<Level2>();

l3s[0].OneToOne_Required_PK = l4s[0];
l3s[1].OneToOne_Required_PK = l4s[1];
Expand Down
2 changes: 1 addition & 1 deletion ...tyFrameworkCore/Query/ExpressionVisitors/Internal/NavigationRewritingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ private Expression RewriteNavigationsIntoJoins(

foreach (var navigation in navigations)
{
if (!navigation.ForeignKey.IsRequired)
if (!navigation.ForeignKey.IsRequired || !navigation.IsDependentToPrincipal())
{
optionalNavigationInChain = true;
}
Expand Down
Loading

0 comments on commit 21dc58c

Please sign in to comment.