Skip to content

Commit efe6aa0

Browse files
alex-kulakovalex-kulakov
authored
Merge pull request #406 from DataObjects-NET/6.0-innerjoin-instead-of-leftjoin-issue
Addresses some cases when INNER JOIN was wrongly chosen instead of LEFT JOIN
2 parents c204cd7 + 2e20609 commit efe6aa0

File tree

6 files changed

+223
-43
lines changed

6 files changed

+223
-43
lines changed

‎ChangeLog/6.0.14_dev.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
[main] Addressed issue of not visiting grouping expression correctly which appeared in previous version
2-
[main] Addressed certain issues of incorrect query optimization when it contains multiple In() calls
2+
[main] Addressed certain issues of incorrect query optimization when it contains multiple In() calls
3+
[main] In addition to previous version, more cases of incorrect join type usage addressed

‎Orm/Xtensive.Orm.Tests/Issues/IssueGithub0281_InnerJoinForNullableEntityReference.cs

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright (C) 2024 Xtensive LLC.
2+
// This code is distributed under MIT license terms.
3+
// See the License.txt file in the project root for more information.
4+
5+
using System.Linq;
6+
using NUnit.Framework;
7+
using Xtensive.Orm.Configuration;
8+
using Xtensive.Orm.Tests.Issues.IssueGithub0281_InnerJoinForNullableEntityReferenceModel;
9+
10+
namespace Xtensive.Orm.Tests.Issues.IssueGithub0281_InnerJoinForNullableEntityReferenceModel
11+
{
12+
[HierarchyRoot]
13+
public class TestEntity : Entity
14+
{
15+
[Field, Key]
16+
public int Id { get; set; }
17+
18+
[Field(Nullable = false)]
19+
public string Name { get; set; }
20+
21+
public TestEntity(Session session)
22+
: base(session)
23+
{
24+
}
25+
}
26+
27+
[HierarchyRoot]
28+
public class TestEntity2 : Entity
29+
{
30+
[Field, Key]
31+
public int Id { get; private set; }
32+
33+
[Field(Nullable = false)]
34+
public string Name { get; set; }
35+
36+
[Field(Nullable = false)]
37+
public TestEntity Owner { get; set; }
38+
39+
[Field]
40+
public TestEntity3 NullableLink { get; set; }
41+
42+
[Field(Nullable = false)]
43+
public TestEntity3 Link { get; set; }
44+
45+
public TestEntity2(Session session)
46+
: base(session)
47+
{
48+
}
49+
}
50+
51+
[HierarchyRoot]
52+
public class TestEntity3 : Entity
53+
{
54+
[Field, Key]
55+
public int Id { get; private set; }
56+
57+
[Field(Nullable = false)]
58+
public string Name { get; set; }
59+
60+
public TestEntity3(Session session)
61+
: base(session)
62+
{
63+
}
64+
}
65+
}
66+
67+
namespace Xtensive.Orm.Tests.Issues
68+
{
69+
public sealed class IssueGithub0281_InnerJoinForNullableEntityReference : AutoBuildTest
70+
{
71+
protected override DomainConfiguration BuildConfiguration()
72+
{
73+
var domainConfiguration = base.BuildConfiguration();
74+
75+
domainConfiguration.Types.Register(typeof(TestEntity));
76+
domainConfiguration.Types.Register(typeof(TestEntity2));
77+
domainConfiguration.Types.Register(typeof(TestEntity3));
78+
79+
domainConfiguration.UpgradeMode = DomainUpgradeMode.Recreate;
80+
81+
return domainConfiguration;
82+
}
83+
84+
protected override void PopulateData()
85+
{
86+
using (var session = Domain.OpenSession())
87+
using (var t = session.OpenTransaction()) {
88+
_ = new TestEntity(session) { Name = "1" };
89+
_ = new TestEntity(session) { Name = "2" };
90+
91+
t.Complete();
92+
}
93+
}
94+
95+
[Test]
96+
public void SimpleFieldTest()
97+
{
98+
using (var session = Domain.OpenSession())
99+
using (session.OpenTransaction()) {
100+
var result = session.Query.All<TestEntity>()
101+
.Select(e => new {
102+
e.Id,
103+
Name = session.Query.All<TestEntity2>()
104+
.FirstOrDefault(it => it.Owner == e).Name
105+
})
106+
.ToList();
107+
108+
Assert.That(result.Count, Is.EqualTo(2));
109+
Assert.That(result.All(x => x.Name == null));
110+
}
111+
}
112+
113+
[Test]
114+
public void NullableReferenceFieldTest()
115+
{
116+
using (var session = Domain.OpenSession())
117+
using (session.OpenTransaction()) {
118+
var result = session.Query.All<TestEntity>()
119+
.Select(e => new {
120+
e.Id,
121+
NullableLink = session.Query.All<TestEntity2>()
122+
.FirstOrDefault(it => it.Owner == e).NullableLink
123+
})
124+
.ToList();
125+
126+
Assert.That(result.Count, Is.EqualTo(2));
127+
Assert.That(result.All(x => x.NullableLink == null));
128+
}
129+
}
130+
131+
[Test]
132+
public void NonNullableReferenceFieldTest()
133+
{
134+
using (var session = Domain.OpenSession())
135+
using (session.OpenTransaction()) {
136+
var result = session.Query.All<TestEntity>()
137+
.Select(e => new {
138+
e.Id,
139+
Link = session.Query.All<TestEntity2>()
140+
.FirstOrDefault(it => it.Owner == e).Link
141+
})
142+
.ToList();
143+
144+
Assert.That(result.Count, Is.EqualTo(2));
145+
Assert.That(result.All(x => x.Link == null));
146+
}
147+
}
148+
}
149+
}

‎Orm/Xtensive.Orm.Tests/Issues/IssueJira0553_IncorrectLeftJoinOnNotNullEntityField.cs

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2014 Xtensive LLC.
1+
// Copyright (C) 2014 Xtensive LLC.
22
// All rights reserved.
33
// For conditions of distribution and use, see license.
44
// Created by: Alexey Kulakov
@@ -106,20 +106,20 @@ protected override void PopulateData()
106106
using (var session = Domain.OpenSession())
107107
using (var t = session.OpenTransaction()) {
108108
var car = new Car();
109-
new EmployeeWithCar { Car = car };
110-
new Employee();
111-
new Employee();
109+
_ = new EmployeeWithCar { Car = car };
110+
_ = new Employee();
111+
_ = new Employee();
112112

113113
var customer = new Customer();
114114
for (var i = 0; i < 10; i++) {
115-
if (i % 2==0) {
115+
if (i % 2 == 0) {
116116
var job = new Job() {
117-
Location = new Location() {
118-
Address = new Address() {
119-
Street = string.Format("{0} street", i + 1.ToString())
120-
}
121-
}
122-
};
117+
Location = new Location() {
118+
Address = new Address() {
119+
Street = string.Format("{0} street", i + 1.ToString())
120+
}
121+
}
122+
};
123123
var invoice = new Invoice() {Customer = customer, Job = job};
124124
}
125125
else {
@@ -189,6 +189,8 @@ public void WorkaroundTest()
189189
e.Id,
190190
Car = c
191191
});
192+
193+
192194
Assert.AreEqual(3, wordaround.Count());
193195
}
194196
}
@@ -199,11 +201,20 @@ public void Test01()
199201
using (var session = Domain.OpenSession())
200202
using (var transaction = session.OpenTransaction()) {
201203
var customer = session.Query.All<Customer>().First();
202-
var result = (from i in session.Query.All<Invoice>()
204+
var results =
205+
(from i in session.Query.All<Invoice>()
203206
from j in session.Query.All<Job>().Where(j => j==i.Job).DefaultIfEmpty()
204207
where i.Customer.Id==customer.Id
205208
select new {i.Id, Location = j!=null && j.Location!=null ? j.Location.Address.Street : ""}).ToList();
206-
Assert.AreEqual(10, result.Count);
209+
210+
var localResults =
211+
(from i in session.Query.All<Invoice>().AsEnumerable()
212+
from j in session.Query.All<Job>().AsEnumerable().Where(j => j == i.Job).DefaultIfEmpty()
213+
where i.Customer.Id == customer.Id
214+
select new { i.Id, Location = j != null && j.Location != null ? j?.Location.Address.Street : "" }).ToList();
215+
216+
Assert.That(localResults.Count, Is.EqualTo(10));
217+
Assert.That(results.Count, Is.EqualTo(localResults.Count));
207218
}
208219
}
209220

@@ -212,25 +223,46 @@ public void Test02()
212223
{
213224
using (var session = Domain.OpenSession())
214225
using (var transaction = session.OpenTransaction()) {
215-
var results = (from i in session.Query.All<Invoice>()
216-
from j in session.Query.All<Job>().Where(j => j==i.Job).DefaultIfEmpty()
217-
select new {i.Id, Location = j.Location}).Where(el => el.Location!=null || string.IsNullOrEmpty(el.Location.Address.Street)).ToList();
218-
Assert.AreEqual(5, results.Count);
226+
var results =
227+
(from i in session.Query.All<Invoice>()
228+
from j in session.Query.All<Job>().Where(j => j == i.Job).DefaultIfEmpty()
229+
select new { i.Id, Location = j.Location })
230+
.Where(el => el.Location != null || string.IsNullOrEmpty(el.Location.Address.Street))
231+
.ToList();
232+
233+
var localResults =
234+
(from i in session.Query.All<Invoice>().AsEnumerable()
235+
from j in session.Query.All<Job>().AsEnumerable().Where(j => j == i.Job).DefaultIfEmpty()
236+
select new { i.Id, Location = j?.Location })
237+
.Where(el => el.Location != null || string.IsNullOrEmpty(el.Location?.Address?.Street)).ToList();
238+
239+
Assert.That(localResults.Count, Is.EqualTo(10));
240+
Assert.That(results.Count, Is.EqualTo(localResults.Count));
219241
}
220242
}
221243

222244
[Test]
223-
public void Test3()
245+
public void Test03()
224246
{
225247
using (var session = Domain.OpenSession())
226248
using (var transaction = session.OpenTransaction()) {
227249
var cusomer = session.Query.All<Customer>().First();
228-
var result = (from i in session.Query.All<Invoice>()
229-
from j in session.Query.All<Job>().Where(j => j==i.Job).DefaultIfEmpty()
230-
from l in session.Query.All<Location>().Where(l => l==j.Location).DefaultIfEmpty()
231-
where i.Customer.Id==cusomer.Id
232-
select new { i.Id, Location = l!=null ? l.Address.Street : "", }).ToList();
233-
Assert.AreEqual(10, result.Count);
250+
var results =
251+
(from i in session.Query.All<Invoice>()
252+
from j in session.Query.All<Job>().Where(j => j == i.Job).DefaultIfEmpty()
253+
from l in session.Query.All<Location>().Where(l => l == j.Location).DefaultIfEmpty()
254+
where i.Customer.Id == cusomer.Id
255+
select new { i.Id, Location = l != null ? l.Address.Street : "", }).ToList();
256+
257+
var localResults =
258+
(from i in session.Query.All<Invoice>().AsEnumerable()
259+
from j in session.Query.All<Job>().AsEnumerable().Where(j => j == i?.Job).DefaultIfEmpty()
260+
from l in session.Query.All<Location>().AsEnumerable().Where(l => l == j?.Location).DefaultIfEmpty()
261+
where i?.Customer?.Id == cusomer.Id
262+
select new { i.Id, Location = l != null ? l.Address.Street : "", }).ToList();
263+
264+
Assert.That(localResults.Count, Is.EqualTo(10));
265+
Assert.That(results.Count, Is.EqualTo(localResults.Count));
234266
}
235267
}
236268
}

‎Orm/Xtensive.Orm/Orm/Linq/Expressions/ItemProjectorExpression.cs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,9 @@ public ItemProjectorExpression EnsureEntityIsJoined()
145145
rightIndex++;
146146
}
147147
var offset = dataSource.Header.Length;
148-
dataSource = entityExpression.IsNullable
149-
|| (dataSource is JoinProvider dataSourceAsJoin && dataSourceAsJoin.JoinType == JoinType.LeftOuter)
150-
? dataSource.LeftJoin(joinedRs, keyPairs)
151-
: dataSource.Join(joinedRs, keyPairs);
148+
dataSource = entityExpression.IsNullable || dataSource.CheckIfLeftJoinPrefered()
149+
? dataSource.LeftJoin(joinedRs, keyPairs)
150+
: dataSource.Join(joinedRs, keyPairs);
152151
EntityExpression.Fill(entityExpression, offset);
153152
return entityExpression;
154153
}
@@ -169,10 +168,9 @@ public ItemProjectorExpression EnsureEntityIsJoined()
169168
rightIndex++;
170169
}
171170
var offset = dataSource.Header.Length;
172-
dataSource = entityFieldExpression.IsNullable
173-
|| (dataSource is JoinProvider dataSourceAsJoin && dataSourceAsJoin.JoinType == JoinType.LeftOuter)
174-
? dataSource.LeftJoin(joinedRs, keyPairs)
175-
: dataSource.Join(joinedRs, keyPairs);
171+
dataSource = entityFieldExpression.IsNullable || dataSource.CheckIfLeftJoinPrefered()
172+
? dataSource.LeftJoin(joinedRs, keyPairs)
173+
: dataSource.Join(joinedRs, keyPairs);
176174
entityFieldExpression.RegisterEntityExpression(offset);
177175
return entityFieldExpression.Entity;
178176
}

‎Orm/Xtensive.Orm/Orm/Linq/Translator.Expressions.cs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,7 +1665,8 @@ public void EnsureEntityFieldsAreJoined(EntityExpression entityExpression, ItemP
16651665
.ToArray();
16661666
int offset = itemProjector.DataSource.Header.Length;
16671667
var oldDataSource = itemProjector.DataSource;
1668-
var newDataSource = entityExpression.IsNullable
1668+
1669+
var newDataSource = entityExpression.IsNullable || oldDataSource.CheckIfLeftJoinPrefered()
16691670
? itemProjector.DataSource.LeftJoin(joinedRs, keyPairs)
16701671
: itemProjector.DataSource.Join(joinedRs, keyPairs);
16711672
itemProjector.DataSource = newDataSource;
@@ -1690,15 +1691,7 @@ private void EnsureEntityReferenceIsJoined(EntityFieldExpression entityFieldExpr
16901691

16911692
var oldDataSource = originalItemProjector.DataSource;
16921693
var offset = oldDataSource.Header.Length;
1693-
var shouldUseLeftJoin = false;
1694-
1695-
var sourceToCheck = (oldDataSource is FilterProvider filterProvider) ? filterProvider.Source : oldDataSource;
1696-
if ((sourceToCheck is ApplyProvider applyProvider && applyProvider.ApplyType == JoinType.LeftOuter) ||
1697-
(sourceToCheck is JoinProvider joinProvider && joinProvider.JoinType == JoinType.LeftOuter)) {
1698-
shouldUseLeftJoin = true;
1699-
}
1700-
1701-
var newDataSource = entityFieldExpression.IsNullable || shouldUseLeftJoin
1694+
var newDataSource = entityFieldExpression.IsNullable || oldDataSource.CheckIfLeftJoinPrefered()
17021695
? oldDataSource.LeftJoin(joinedRs, keyPairs)
17031696
: oldDataSource.Join(joinedRs, keyPairs);
17041697
originalItemProjector.DataSource = newDataSource;

‎Orm/Xtensive.Orm/Orm/Rse/CompilableProviderExtensions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,5 +212,12 @@ public static CompilableProvider MakeVoid(this CompilableProvider source)
212212
{
213213
return new VoidProvider(source.Header);
214214
}
215+
216+
internal static bool CheckIfLeftJoinPrefered(this CompilableProvider provider)
217+
{
218+
var sourceToCheck = (provider is FilterProvider filterProvider) ? filterProvider.Source : provider;
219+
return (sourceToCheck is ApplyProvider applyProvider && applyProvider.ApplyType == JoinType.LeftOuter) ||
220+
(sourceToCheck is JoinProvider joinProvider && joinProvider.JoinType == JoinType.LeftOuter);
221+
}
215222
}
216223
}

0 commit comments

Comments
 (0)