Skip to content

Skip no longer needed moving ON condition to Where clause in LINQ #2539

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

Merged
merged 2 commits into from
Sep 15, 2020
Merged

Skip no longer needed moving ON condition to Where clause in LINQ #2539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89f8755
Skip no longer needed moving ON condition to Where clause in LINQ
bahusoid Sep 15, 2020
a3bc6bb
Fix test
bahusoid Sep 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,51 +1078,40 @@ from c in db.Customers
}

[Category("JOIN")]
[TestCase(true, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
[TestCase(false, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
public async Task DLinqJoin5dAsync(bool useCrossJoin)
[Test(Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
public async Task DLinqJoin5dAsync()
{
if (useCrossJoin && !Dialect.SupportsCrossJoin)
{
Assert.Ignore("Dialect does not support cross join.");
}

var q =
from c in db.Customers
join o in db.Orders on
new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null }
select new { c.ContactName, o.OrderId };

using (var substitute = SubstituteDialect())
using (var sqlSpy = new SqlLogSpy())
{
ClearQueryPlanCache();
substitute.Value.SupportsCrossJoin.Returns(useCrossJoin);

await (ObjectDumper.WriteAsync(q));

var sql = sqlSpy.GetWholeLog();
Assert.That(sql, Does.Contain(useCrossJoin ? "cross join" : "inner join"));
Assert.That(GetTotalOccurrences(sql, "left outer join"), Is.EqualTo(0));
Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(useCrossJoin ? 1 : 2));
Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(useCrossJoin ? 1 : 0));
Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(2));
Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(0));
}
}

[Category("JOIN")]
[Test(Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
public void DLinqJoin5dLeftJoinAsync()
public async Task DLinqJoin5dLeftJoinAsync()
{
var q =
from c in db.Customers
join o in db.Orders on
new { c.CustomerId, HasContractTitle = c.ContactTitle != null } equals
new { o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null } into orders
new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null} into orders
from o in orders.DefaultIfEmpty()
select new { c.ContactName, o.OrderId };
select new {c.ContactName, OrderId = (int?) o.OrderId};

Assert.ThrowsAsync<NotSupportedException>(() => ObjectDumper.WriteAsync(q));
await (ObjectDumper.WriteAsync(q));
}

[Category("JOIN")]
Expand Down
27 changes: 8 additions & 19 deletions src/NHibernate.Test/Linq/LinqQuerySamples.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1622,35 +1622,24 @@ from c in db.Customers
}

[Category("JOIN")]
[TestCase(true, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
[TestCase(false, Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
public void DLinqJoin5d(bool useCrossJoin)
[Test(Description = "This sample explictly joins two tables with a composite key and projects results from both tables.")]
public void DLinqJoin5d()
{
if (useCrossJoin && !Dialect.SupportsCrossJoin)
{
Assert.Ignore("Dialect does not support cross join.");
}

var q =
from c in db.Customers
join o in db.Orders on
new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null }
select new { c.ContactName, o.OrderId };

using (var substitute = SubstituteDialect())
using (var sqlSpy = new SqlLogSpy())
{
ClearQueryPlanCache();
substitute.Value.SupportsCrossJoin.Returns(useCrossJoin);

ObjectDumper.Write(q);

var sql = sqlSpy.GetWholeLog();
Assert.That(sql, Does.Contain(useCrossJoin ? "cross join" : "inner join"));
Assert.That(GetTotalOccurrences(sql, "left outer join"), Is.EqualTo(0));
Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(useCrossJoin ? 1 : 2));
Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(useCrossJoin ? 1 : 0));
Assert.That(GetTotalOccurrences(sql, "inner join"), Is.EqualTo(2));
Assert.That(GetTotalOccurrences(sql, "cross join"), Is.EqualTo(0));
}
}

Expand All @@ -1661,12 +1650,12 @@ public void DLinqJoin5dLeftJoin()
var q =
from c in db.Customers
join o in db.Orders on
new { c.CustomerId, HasContractTitle = c.ContactTitle != null } equals
new { o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null } into orders
new {c.CustomerId, HasContractTitle = c.ContactTitle != null} equals
new {o.Customer.CustomerId, HasContractTitle = o.Customer.ContactTitle != null} into orders
from o in orders.DefaultIfEmpty()
select new { c.ContactName, o.OrderId };
select new {c.ContactName, OrderId = (int?) o.OrderId};

Assert.Throws<NotSupportedException>(() => ObjectDumper.Write(q));
ObjectDumper.Write(q);
}

[Category("JOIN")]
Expand Down
37 changes: 4 additions & 33 deletions src/NHibernate/Linq/ReWriters/AddJoinsReWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;
using System.Linq.Expressions;
using NHibernate.Engine;
Expand All @@ -22,13 +20,11 @@ public class AddJoinsReWriter : NhQueryModelVisitorBase, IIsEntityDecider, INhQu
private readonly ISessionFactoryImplementor _sessionFactory;
private readonly MemberExpressionJoinDetector _memberExpressionJoinDetector;
private readonly WhereJoinDetector _whereJoinDetector;
private JoinClause _currentJoin;
private bool? _innerJoin;

private AddJoinsReWriter(ISessionFactoryImplementor sessionFactory, QueryModel queryModel)
{
_sessionFactory = sessionFactory;
var joiner = new Joiner(queryModel, AddJoin);
var joiner = new Joiner(queryModel);
_memberExpressionJoinDetector = new MemberExpressionJoinDetector(this, joiner, _sessionFactory);
_whereJoinDetector = new WhereJoinDetector(this, joiner, _sessionFactory);
}
Expand Down Expand Up @@ -66,30 +62,17 @@ public override void VisitNhHavingClause(NhHavingClause havingClause, QueryModel

public void VisitNhOuterJoinClause(NhOuterJoinClause nhOuterJoinClause, QueryModel queryModel, int index)
{
VisitJoinClause(nhOuterJoinClause.JoinClause, false);
VisitJoinClause(nhOuterJoinClause.JoinClause);
}

public override void VisitJoinClause(JoinClause joinClause, QueryModel queryModel, int index)
{
VisitJoinClause(joinClause, true);
VisitJoinClause(joinClause);
}

private void VisitJoinClause(JoinClause joinClause, bool innerJoin)
private void VisitJoinClause(JoinClause joinClause)
{
joinClause.InnerSequence = _whereJoinDetector.Transform(joinClause.InnerSequence);

// When associations are located in the outer key (e.g. from a in A join b in B b on a.C.D.Id equals b.Id),
// do nothing and leave them to HQL for adding the missing joins.

// When associations are located in the inner key (e.g. from a in A join b in B b on a.Id equals b.C.D.Id),
// we have to move the condition to the where statement, otherwise the query will be invalid (HQL does not
// support them).
// Link newly created joins with the current join clause in order to later detect which join type to use.
_currentJoin = joinClause;
_innerJoin = innerJoin;
joinClause.InnerKeySelector = _whereJoinDetector.Transform(joinClause.InnerKeySelector);
_currentJoin = null;
_innerJoin = null;
}

// Since v5.3
Expand All @@ -107,18 +90,6 @@ public bool IsIdentifier(System.Type type, string propertyName)
return metadata != null && propertyName.Equals(metadata.IdentifierPropertyName);
}

private void AddJoin(QueryModel queryModel, NhJoinClause joinClause)
{
joinClause.ParentJoinClause = _currentJoin;
if (_innerJoin == true)
{
// Match the parent join type
joinClause.MakeInner();
}

queryModel.BodyClauses.Add(joinClause);
}

bool IIsEntityDecider.IsEntity(MemberExpression expression, out bool isIdentifier)
{
isIdentifier =
Expand Down
6 changes: 0 additions & 6 deletions src/NHibernate/Linq/Visitors/JoinBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,6 @@ public class Joiner : IJoiner
private readonly NameGenerator _nameGenerator;
private readonly QueryModel _queryModel;

internal Joiner(QueryModel queryModel, System.Action<QueryModel, NhJoinClause> addJoinMethod)
: this(queryModel)
{
AddJoinMethod = addJoinMethod;
}

internal Joiner(QueryModel queryModel)
{
_nameGenerator = new NameGenerator(queryModel);
Expand Down