Support LINQ GroupBy subquery #2210
Conversation
| switch (resultOperator) | ||
| { | ||
| case ContainsResultOperator _: | ||
| case AnyResultOperator _: |
There was a problem hiding this comment.
AnyResultOperator actually currently doesn't throw on type mismatch. But it looks like omission in ReLinq as I see no reasons why it should behave any differently. So added it just in case if it's changed in ReLinq.
| //at Remotion.Linq.Clauses.ResultOperators.ContainsResultOperator.GetOutputDataInfo(StreamedSequenceInfo inputInfo) | ||
| //But in this place we don't really care about types involving inside expression, all we need to know is operation result which is bool for Contains | ||
| //So let's skip possible type exception mismatch if it allows to generate proper SQL | ||
| switch (resultOperator) |
There was a problem hiding this comment.
Instead of doing it here, can we fix the condition at its source?
There was a problem hiding this comment.
I don't really understand the real source of the issue. You should know this parts much better than me. And as this issue hangs since 2012 I think it's worth to implement such simple workaround.
There was a problem hiding this comment.
The issue is removing MaxResultOperator and replacing it with NhMaxExpression.
There was a problem hiding this comment.
What about Min and others aggregate functions? It's Group By specific not Max specific.
There was a problem hiding this comment.
That's exactly the reason not to do it. To keep workaround in one place and it doesn't affect perf
(instead of type integrity check inside we do this resultOperator check)
There was a problem hiding this comment.
If you are suggesting that changing NhMaxExpression and etc. will fix issue at its core and it won't be workaround than please make those changes yourself and I will close this PR. As I still don't understand what fix exactly do you have in mind.
There was a problem hiding this comment.
I'm probably fine with the proposed change.
fredericDelaporte
added
t: Fix
and removed
c: Linq Provider
p: Minor
t: Improvement
labels
Fixes #1124
Yet another workaround to make LINQ GroupBy subquery works. IMHO it's less intrusive than last @oskarb attempt in https://github.com/oskarb/nhibernate-core/tree/NH-3155-alt2. And doesn't look very hackish to me - allows type mismatch and doesn't fail in specific places.