Computed GroupBys don't work if they have constants
Description
Environment
Activity
Closing issues resolved in 4.1.0
Restructured the code in the pull request a little to make it cleaner and easier to reuse.
The strategy looks good to me. The context passed between the queries could be handled a little more generically IMHO, but I'm not picky about it.
I can see the change modifies the public API... why are we exposing all the rewriters publicly anyways?
Whoever reviews the pull request, please indicate if anything may not be acceptable. We have great need for this feature so we're willing to put in the time to make it happen.
Since assuming that all constants are acceptable for projecting into the select statement is way too permissive, I took another route.
Instead I modify the QueryModelVisitor to use the group by key expressions as first-class candidates for Select expression re-writing. This hinges on the fact that the group by key expressions are going to be generating HQL anyway, so we ought to be able to use them in the select statement.
https://github.com/nhibernate/nhibernate-core/pull/432
Okay... I've done some digging and I found a simple fix...
The example query generates the following SQL
The SelectClauseHqlNominator does not nominate the full ternary expression as a candidate for the select clause.
As it traverses the expression tree IIF(([o].Customer.CustomerId == null), 0, 1) it seems to discard all of the constant values and all of the expressions that depend on constant values. So the top level expression isn't considered as a candidate for replacement.
By allowing the SelectClauseHqlNominator to project constants into ExpressionType.New expressions, there is better coverage and the following SQL is generated
Details
Details
Assignee
Reporter
Components
Fix versions
Affects versions
Priority
Major
You can see this fail by adding this test to NHibernate.Test.Linq.ByMethod.GroupByTests
As a proof of concept, the follow change makes it work:
If you have something like
or
, there must be some logic somewhere that decides "x.A + 1" and "x.B" will end up as separate elements in the group by clause. We could use the same logic to control whether the constants are mapped into the query or not. Basically, whatever is sent to the database for the group-by MUST be the same thing we send in the select clause or the DB will freak out.
I was thinking it would be possible to change the code where it rewrites the select clause based on the group key such that it stores some extra context to tell the select constant part that the expression its working on must be fully translated to HQL.
You can run the example query in a debugger to see what happens. If you watch what happens to "queryModel" as it goes through the NHibernate.Linq.Visitors.QueryModelVisitor you can see it replace the ".Key" with the clause from the GroupBy. It finally decides to "do the wrong thing" in the SelectClauseNominator mentioned above.