-
Notifications
You must be signed in to change notification settings - Fork 934
NH-3474 - Fix support of GroupBy with constant keys #434
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -549,6 +549,100 @@ public void GroupByComputedValueInObjectArray() | |
Assert.AreEqual(830, orderGroups.Sum(g => g.Count)); | ||
} | ||
|
||
[Test(Description = "NH-3474")] | ||
public void GroupByConstant() | ||
{ | ||
var totals = db.Orders.GroupBy(o => 1).Select(g => new { Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight) }).ToList(); | ||
Assert.That(totals.Count, Is.EqualTo(1)); | ||
Assert.That(totals, Has.All.With.Property("Key").EqualTo(1)); | ||
} | ||
|
||
[Test(Description = "NH-3474")] | ||
public void GroupByConstantAnonymousType() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
var totals = db.Orders.GroupBy(o => new { A = 1 }).Select(g => new { Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight) }).ToList(); | ||
Assert.That(totals.Count, Is.EqualTo(1)); | ||
Assert.That(totals, Has.All.With.Property("Key").With.Property("A").EqualTo(1)); | ||
} | ||
|
||
[Test(Description = "NH-3474")] | ||
public void GroupByConstantArray() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
var totals = db.Orders.GroupBy(o => new object[] { 1 }).Select(g => new { Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight) }).ToList(); | ||
Assert.That(totals.Count, Is.EqualTo(1)); | ||
Assert.That(totals, Has.All.With.Property("Key").EqualTo(new object[] { 1 })); | ||
} | ||
|
||
[Test(Description = "NH-3474")] | ||
public void GroupByKeyWithConstantInAnonymousType() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
var totals = db.Orders.GroupBy(o => new { A = 1, B = o.Shipper.ShipperId }).Select(g => new { Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight) }).ToList(); | ||
Assert.That(totals.Count, Is.EqualTo(3)); | ||
Assert.That(totals, Has.All.With.Property("Key").With.Property("A").EqualTo(1)); | ||
} | ||
|
||
[Test(Description = "NH-3474")] | ||
public void GroupByKeyWithConstantInArray() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
var totals = db.Orders.GroupBy(o => new[] { 1, o.Shipper.ShipperId }).Select(g => new { Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight) }).ToList(); | ||
Assert.That(totals.Count, Is.EqualTo(3)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What sql/hql is generated in each of these cases? |
||
Assert.That(totals, Has.All.With.Property("Key").Contains(1)); | ||
} | ||
|
||
private int constKey; | ||
[Test(Description = "NH-3474")] | ||
public void GroupByKeyWithConstantFromVariable() | ||
{ | ||
constKey = 1; | ||
var q1 = db.Orders.GroupBy(o => constKey).Select(g => new {Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight)}); | ||
var q2 = db.Orders.GroupBy(o => new {A = constKey}).Select(g => new {Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight)}); | ||
var q3 = db.Orders.GroupBy(o => new object[] {constKey}).Select(g => new {Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight)}); | ||
var q4 = db.Orders.GroupBy(o => new {A = constKey, B = o.Shipper.ShipperId}).Select(g => new {Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight)}); | ||
var q5 = db.Orders.GroupBy(o => new[] {constKey, o.Shipper.ShipperId}).Select(g => new {Key = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Freight)}); | ||
|
||
var r1_1 = q1.ToList(); | ||
Assert.That(r1_1.Count, Is.EqualTo(1)); | ||
Assert.That(r1_1, Has.All.With.Property("Key").EqualTo(1)); | ||
|
||
var r2_1 = q2.ToList(); | ||
Assert.That(r2_1.Count, Is.EqualTo(1)); | ||
Assert.That(r2_1, Has.All.With.Property("Key").With.Property("A").EqualTo(1)); | ||
|
||
var r3_1 = q3.ToList(); | ||
Assert.That(r3_1.Count, Is.EqualTo(1)); | ||
Assert.That(r3_1, Has.All.With.Property("Key").EqualTo(new object[] { 1 })); | ||
|
||
var r4_1 = q4.ToList(); | ||
Assert.That(r4_1.Count, Is.EqualTo(3)); | ||
Assert.That(r4_1, Has.All.With.Property("Key").With.Property("A").EqualTo(1)); | ||
|
||
var r5_1 = q5.ToList(); | ||
Assert.That(r5_1.Count, Is.EqualTo(3)); | ||
Assert.That(r5_1, Has.All.With.Property("Key").Contains(1)); | ||
|
||
constKey = 2; | ||
|
||
var r1_2 = q1.ToList(); | ||
Assert.That(r1_2.Count, Is.EqualTo(1)); | ||
Assert.That(r1_2, Has.All.With.Property("Key").EqualTo(2)); | ||
|
||
var r2_2 = q2.ToList(); | ||
Assert.That(r2_2.Count, Is.EqualTo(1)); | ||
Assert.That(r2_2, Has.All.With.Property("Key").With.Property("A").EqualTo(2)); | ||
|
||
var r3_2 = q3.ToList(); | ||
Assert.That(r3_2.Count, Is.EqualTo(1)); | ||
Assert.That(r3_2, Has.All.With.Property("Key").EqualTo(new object[] { constKey })); | ||
|
||
var r4_2 = q4.ToList(); | ||
Assert.That(r4_2.Count, Is.EqualTo(3)); | ||
Assert.That(r4_2, Has.All.With.Property("Key").With.Property("A").EqualTo(2)); | ||
|
||
var r5_2 = q5.ToList(); | ||
Assert.That(r5_2.Count, Is.EqualTo(3)); | ||
Assert.That(r5_2, Has.All.With.Property("Key").Contains(2)); | ||
} | ||
|
||
[Test(Description = "NH-3801")] | ||
public void GroupByComputedValueWithJoinOnObject() | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Linq.Expressions; | ||
using NHibernate.Linq.Clauses; | ||
using NHibernate.Linq.ReWriters; | ||
using NHibernate.Linq.Visitors; | ||
|
@@ -58,6 +59,7 @@ public static void ReWrite(QueryModel queryModel) | |
if (groupBy != null) | ||
{ | ||
FlattenSubQuery(queryModel, subQueryExpression.QueryModel, groupBy); | ||
RemoveCostantGroupByKeys(queryModel, groupBy); | ||
} | ||
} | ||
} | ||
|
@@ -101,5 +103,22 @@ private static void FlattenSubQuery(QueryModel queryModel, QueryModel subQueryMo | |
// Replace the outer query source | ||
queryModel.MainFromClause = subQueryModel.MainFromClause; | ||
} | ||
|
||
private static void RemoveCostantGroupByKeys(QueryModel queryModel, GroupResultOperator groupBy) | ||
{ | ||
var keys = groupBy.ExtractKeyExpressions().Where(x => !(x is ConstantExpression)).ToList(); | ||
|
||
if (!keys.Any()) | ||
{ | ||
// Remove the Group By clause completely if all the keys are constant (redundant) | ||
queryModel.ResultOperators.Remove(groupBy); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please include test for NH-2500 (overcaching) in conclusion with this feature. It might be the case. |
||
} | ||
else | ||
{ | ||
// Re-write the KeySelector as an object array of the non-constant keys | ||
// This should be safe because we've already re-written the select clause using the original keys | ||
groupBy.KeySelector = Expression.NewArrayInit(typeof (object), keys.Select(x => x.Type.IsValueType ? Expression.Convert(x, typeof(object)) : x)); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Linq.Expressions; | ||
using System.Text; | ||
using NHibernate.Util; | ||
using Remotion.Linq.Clauses.ResultOperators; | ||
|
||
namespace NHibernate.Linq | ||
{ | ||
internal static class GroupResultOperatorExtensions | ||
{ | ||
public static IEnumerable<Expression> ExtractKeyExpressions(this GroupResultOperator groupResult) | ||
{ | ||
if (groupResult.KeySelector is NewExpression) | ||
return (groupResult.KeySelector as NewExpression).Arguments; | ||
if (groupResult.KeySelector is NewArrayExpression) | ||
return (groupResult.KeySelector as NewArrayExpression).Expressions; | ||
return new [] { groupResult.KeySelector }; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generates
select
cast(count(*) as INT) as col_0_0_,
cast(sum(order0_.Freight) as DECIMAL(19,
5)) as col_1_0_
from
Orders order0_