-
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
Conversation
PleasantD
commented
Jun 24, 2015
- Remove the GROUP BY clause entirely if all keys are constant
- Re-write the GROUP BY clause using only non-constant keys
@hazzik It would be very beneficial if this fix could be included in v4.1.0 |
FlattenSubQuery(queryModel, subQueryExpression.QueryModel, groupBy); | ||
RemoveCostantGroupByKeys(queryModel, groupBy).ForEach(groupByKeys.Add); |
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.
Can you please rewrite this to regular foreach
?
@@ -549,6 +549,46 @@ public void GroupByComputedValueInObjectArray() | |||
Assert.AreEqual(830, orderGroups.Sum(g => g.Count)); | |||
} | |||
|
|||
[Test(Description = "NH-3474")] | |||
public void GroupByConstant() |
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_
Fixed the overcaching issue and fixed other comments. |
@PleasantD can you please squash the PR and rebase on master? |
* Remove the GROUP BY clause entirely if all keys are constant * Re-write the GROUP BY clause using only non-constant keys * Adjust the ExpressionKeyVisitor so that constants in GroupBy are not replaced with parameter * Fix ExpressionKeyVisitor handling of constant arrays to show initialization values
I've added some more tests and merged. Thanks |