Skip to content

Add cross join support for Hql and Linq query provider #2327

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

Merged
merged 10 commits into from
Mar 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
21 changes: 21 additions & 0 deletions src/NHibernate.Test/Async/Hql/EntityJoinHqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,27 @@ public async Task EntityJoinWithFetchesAsync()
}
}

[Test]
public async Task CrossJoinAndWhereClauseAsync()
{
using (var sqlLog = new SqlLogSpy())
using (var session = OpenSession())
{
var result = await (session.CreateQuery(
"SELECT s " +
"FROM EntityComplex s cross join EntityComplex q " +
"where s.SameTypeChild.Id = q.SameTypeChild.Id"
).ListAsync());

Assert.That(result, Has.Count.EqualTo(1));
Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected");
if (Dialect.SupportsCrossJoin)
{
Assert.That(sqlLog.GetWholeLog(), Does.Contain("cross join"), "A cross join is expected in the SQL select");
}
}
}

#region Test Setup

protected override HbmMapping GetMappings()
Expand Down
26 changes: 26 additions & 0 deletions src/NHibernate.Test/Async/Linq/ByMethod/JoinTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,31 @@ public async Task MultipleLinqJoinsWithSameProjectionNamesAsync()
Assert.That(orders.Count, Is.EqualTo(828));
Assert.IsTrue(orders.All(x => x.FirstId == x.SecondId - 1 && x.SecondId == x.ThirdId - 1));
}

[Test]
public async Task CrossJoinWithPredicateInOnStatementAsync()
{
var result =
await ((from o in db.Orders
from p in db.Products
join d in db.OrderLines
on new { o.OrderId, p.ProductId } equals new { d.Order.OrderId, d.Product.ProductId }
into details
from d in details
select new { o.OrderId, p.ProductId, d.UnitPrice }).Take(10).ToListAsync());

Assert.That(result.Count, Is.EqualTo(10));
}

[Test]
public async Task CrossJoinWithPredicateInWhereStatementAsync()
{
var result = await ((from o in db.Orders
from o2 in db.Orders.Where(x => x.Freight > 50)
where (o.OrderId == o2.OrderId + 1) || (o.OrderId == o2.OrderId - 1)
select new { o.OrderId, OrderId2 = o2.OrderId }).ToListAsync());

Assert.That(result.Count, Is.EqualTo(720));
}
}
}
23 changes: 22 additions & 1 deletion src/NHibernate.Test/Hql/EntityJoinHqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ public void EntityJoinWithFetches()
}

[Test, Ignore("Failing for unrelated reasons")]
public void CrossJoinAndWithClause()
public void ImplicitJoinAndWithClause()
{
//This is about complex theta style join fix that was implemented in hibernate along with entity join functionality
//https://hibernate.atlassian.net/browse/HHH-7321
Expand All @@ -279,6 +279,27 @@ public void CrossJoinAndWithClause()
}
}

[Test]
public void CrossJoinAndWhereClause()
{
using (var sqlLog = new SqlLogSpy())
using (var session = OpenSession())
{
var result = session.CreateQuery(
"SELECT s " +
"FROM EntityComplex s cross join EntityComplex q " +
"where s.SameTypeChild.Id = q.SameTypeChild.Id"
).List();

Assert.That(result, Has.Count.EqualTo(1));
Assert.That(sqlLog.Appender.GetEvents().Length, Is.EqualTo(1), "Only one SQL select is expected");
if (Dialect.SupportsCrossJoin)
{
Assert.That(sqlLog.GetWholeLog(), Does.Contain("cross join"), "A cross join is expected in the SQL select");
}
}
}

#region Test Setup

protected override HbmMapping GetMappings()
Expand Down
26 changes: 26 additions & 0 deletions src/NHibernate.Test/Linq/ByMethod/JoinTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,31 @@ public void MultipleLinqJoinsWithSameProjectionNames()
Assert.That(orders.Count, Is.EqualTo(828));
Assert.IsTrue(orders.All(x => x.FirstId == x.SecondId - 1 && x.SecondId == x.ThirdId - 1));
}

[Test]
public void CrossJoinWithPredicateInOnStatement()
{
var result =
(from o in db.Orders
from p in db.Products
join d in db.OrderLines
on new { o.OrderId, p.ProductId } equals new { d.Order.OrderId, d.Product.ProductId }
into details
from d in details
select new { o.OrderId, p.ProductId, d.UnitPrice }).Take(10).ToList();

Assert.That(result.Count, Is.EqualTo(10));
}

[Test]
public void CrossJoinWithPredicateInWhereStatement()
{
var result = (from o in db.Orders
from o2 in db.Orders.Where(x => x.Freight > 50)
where (o.OrderId == o2.OrderId + 1) || (o.OrderId == o2.OrderId - 1)
select new { o.OrderId, OrderId2 = o2.OrderId }).ToList();

Assert.That(result.Count, Is.EqualTo(720));
}
}
}
1 change: 1 addition & 0 deletions src/NHibernate/AdoNet/Util/BasicFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ static BasicFormatter()
{
beginClauses.Add("left");
beginClauses.Add("right");
beginClauses.Add("cross");
beginClauses.Add("inner");
beginClauses.Add("outer");
beginClauses.Add("group");
Expand Down
3 changes: 3 additions & 0 deletions src/NHibernate/Dialect/DB2Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@ public override string ForUpdateString

public override bool SupportsResultSetPositionQueryMethodsOnForwardOnlyCursor => false;

/// <inheritdoc />
public override bool SupportsCrossJoin => false; // DB2 v9.1 doesn't support 'cross join' syntax

public override bool SupportsLobValueChangePropogation => false;

public override bool SupportsExistsInSelect => false;
Expand Down
5 changes: 5 additions & 0 deletions src/NHibernate/Dialect/Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,11 @@ public virtual JoinFragment CreateOuterJoinFragment()
return new ANSIJoinFragment();
}

/// <summary>
/// Does this dialect support CROSS JOIN?
/// </summary>
public virtual bool SupportsCrossJoin => true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be false by default? Otherwise it can be breaking change for some user custom dialects

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the same approach Hibernate did with getCrossJoinSeparator which enables cross join by default. If you think that is better to have it disabled by default I will do it, otherwise we can add it as a possible breaking change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with

add it as a possible breaking change.


/// <summary>
/// Create a <see cref="CaseFragment"/> strategy responsible
/// for handling this dialect's variations in how CASE statements are
Expand Down
5 changes: 4 additions & 1 deletion src/NHibernate/Dialect/InformixDialect0940.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ public override JoinFragment CreateOuterJoinFragment()
return new ANSIJoinFragment();
}

/// <summary>
/// <inheritdoc />
public override bool SupportsCrossJoin => false;

/// <summary>
/// Does this Dialect have some kind of <c>LIMIT</c> syntax?
/// </summary>
/// <value>False, unless overridden.</value>
Expand Down
3 changes: 3 additions & 0 deletions src/NHibernate/Dialect/Oracle10gDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@ public override JoinFragment CreateOuterJoinFragment()
{
return new ANSIJoinFragment();
}

/// <inheritdoc />
public override bool SupportsCrossJoin => true;
}
}
3 changes: 3 additions & 0 deletions src/NHibernate/Dialect/Oracle8iDialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ public override JoinFragment CreateOuterJoinFragment()
return new OracleJoinFragment();
}

/// <inheritdoc />
public override bool SupportsCrossJoin => false;

/// <summary>
/// Map case support to the Oracle DECODE function. Oracle did not
/// add support for CASE until 9i.
Expand Down
3 changes: 3 additions & 0 deletions src/NHibernate/Dialect/SybaseASA9Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ public override bool OffsetStartsAtOne
get { return true; }
}

/// <inheritdoc />
public override bool SupportsCrossJoin => false;

public override SqlString GetLimitString(SqlString queryString, SqlString offset, SqlString limit)
{
int intSelectInsertPoint = GetAfterSelectInsertPoint(queryString);
Expand Down
5 changes: 4 additions & 1 deletion src/NHibernate/Dialect/SybaseASE15Dialect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,10 @@ public override bool SupportsExpectedLobUsagePattern
{
get { return false; }
}


/// <inheritdoc />
public override bool SupportsCrossJoin => false;

public override char OpenQuote
{
get { return '['; }
Expand Down
2 changes: 2 additions & 0 deletions src/NHibernate/Hql/Ast/ANTLR/Hql.g
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ tokens
BETWEEN='between';
CLASS='class';
COUNT='count';
CROSS='cross';
DELETE='delete';
DESCENDING='desc';
DOT;
Expand Down Expand Up @@ -255,6 +256,7 @@ fromClause
fromJoin
: ( ( ( LEFT | RIGHT ) (OUTER)? ) | FULL | INNER )? JOIN^ (FETCH)? path (asAlias)? (propertyFetch)? (withClause)?
| ( ( ( LEFT | RIGHT ) (OUTER)? ) | FULL | INNER )? JOIN^ (FETCH)? ELEMENTS! OPEN! path CLOSE! (asAlias)? (propertyFetch)? (withClause)?
| CROSS JOIN^ { WeakKeywords(); } path (asAlias)?
;

withClause
Expand Down
3 changes: 3 additions & 0 deletions src/NHibernate/Hql/Ast/ANTLR/HqlSqlWalker.g
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,9 @@ joinType returns [int j]
| INNER {
$j = INNER;
}
| CROSS {
$j = CROSS;
}
;

// Matches a path and returns the normalized string for the path (usually
Expand Down
2 changes: 2 additions & 0 deletions src/NHibernate/Hql/Ast/ANTLR/Util/JoinProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public static JoinType ToHibernateJoinType(int astJoinType)
return JoinType.RightOuterJoin;
case HqlSqlWalker.FULL:
return JoinType.FullJoin;
case HqlSqlWalker.CROSS:
return JoinType.CrossJoin;
default:
throw new AssertionFailure("undefined join type " + astJoinType);
}
Expand Down
5 changes: 5 additions & 0 deletions src/NHibernate/Hql/Ast/HqlTreeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ public HqlLeftJoin LeftJoin(HqlExpression expression, HqlAlias @alias)
return new HqlLeftJoin(_factory, expression, @alias);
}

public HqlCrossJoin CrossJoin(HqlExpression expression, HqlAlias @alias)
{
return new HqlCrossJoin(_factory, expression, @alias);
}

public HqlFetchJoin FetchJoin(HqlExpression expression, HqlAlias @alias)
{
return new HqlFetchJoin(_factory, expression, @alias);
Expand Down
15 changes: 15 additions & 0 deletions src/NHibernate/Hql/Ast/HqlTreeNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,13 @@ public class HqlLeftJoin : HqlTreeNode
}
}

public class HqlCrossJoin : HqlTreeNode
{
public HqlCrossJoin(IASTFactory factory, HqlExpression expression, HqlAlias @alias) : base(HqlSqlWalker.JOIN, "join", factory, new HqlCross(factory), expression, @alias)
{
}
}

public class HqlFetchJoin : HqlTreeNode
{
public HqlFetchJoin(IASTFactory factory, HqlExpression expression, HqlAlias @alias)
Expand Down Expand Up @@ -906,6 +913,14 @@ public HqlLeft(IASTFactory factory)
}
}

public class HqlCross : HqlTreeNode
{
public HqlCross(IASTFactory factory)
: base(HqlSqlWalker.CROSS, "cross", factory)
{
}
}

public class HqlAny : HqlBooleanExpression
{
public HqlAny(IASTFactory factory) : base(HqlSqlWalker.ANY, "any", factory)
Expand Down
18 changes: 8 additions & 10 deletions src/NHibernate/Linq/Visitors/QueryModelVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -315,22 +315,20 @@ public override void VisitMainFromClause(MainFromClause fromClause, QueryModel q
public override void VisitAdditionalFromClause(AdditionalFromClause fromClause, QueryModel queryModel, int index)
{
var querySourceName = VisitorParameters.QuerySourceNamer.GetName(fromClause);

var fromExpressionTree = HqlGeneratorExpressionVisitor.Visit(fromClause.FromExpression, VisitorParameters);
var alias = _hqlTree.TreeBuilder.Alias(querySourceName);
if (fromClause.FromExpression is MemberExpression)
{
// It's a join
_hqlTree.AddFromClause(
_hqlTree.TreeBuilder.Join(
HqlGeneratorExpressionVisitor.Visit(fromClause.FromExpression, VisitorParameters).AsExpression(),
_hqlTree.TreeBuilder.Alias(querySourceName)));
_hqlTree.AddFromClause(_hqlTree.TreeBuilder.Join(fromExpressionTree.AsExpression(), alias));
}
else
{
// TODO - exact same code as in MainFromClause; refactor this out
_hqlTree.AddFromClause(
_hqlTree.TreeBuilder.Range(
HqlGeneratorExpressionVisitor.Visit(fromClause.FromExpression, VisitorParameters),
_hqlTree.TreeBuilder.Alias(querySourceName)));
var join = VisitorParameters.SessionFactory.Dialect.SupportsCrossJoin
? _hqlTree.TreeBuilder.CrossJoin(fromExpressionTree.AsExpression(), alias)
: (HqlTreeNode) _hqlTree.TreeBuilder.Range(fromExpressionTree, alias);

_hqlTree.AddFromClause(join);
}

base.VisitAdditionalFromClause(fromClause, queryModel, index);
Expand Down
11 changes: 10 additions & 1 deletion src/NHibernate/SqlCommand/ANSIJoinFragment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,21 @@ public override void AddJoin(string tableName, string alias, string[] fkColumns,
case JoinType.FullJoin:
joinString = " full outer join ";
break;
case JoinType.CrossJoin:
joinString = " cross join ";
break;
default:
throw new AssertionFailure("undefined join type");
}

_fromFragment.Add(joinString + tableName + ' ' + alias + " on ");
_fromFragment.Add(joinString).Add(tableName).Add(" ").Add(alias).Add(" ");
if (joinType == JoinType.CrossJoin)
{
// Cross join does not have an 'on' statement
return;
}

_fromFragment.Add("on ");
if (fkColumns.Length == 0)
{
AddBareCondition(_fromFragment, on);
Expand Down
3 changes: 2 additions & 1 deletion src/NHibernate/SqlCommand/JoinFragment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ public enum JoinType
InnerJoin = 0,
FullJoin = 4,
LeftOuterJoin = 1,
RightOuterJoin = 2
RightOuterJoin = 2,
CrossJoin = 8
}

/// <summary>
Expand Down