Fix constant value in function cached in LINQ query #1540
Conversation
| insideSelectClause = false; | ||
| break; | ||
| } | ||
| if (expression.Method.DeclaringType?.Namespace == "System.Linq") |
There was a problem hiding this comment.
For putting what in it? All the point of that if is to not alter insideSelectClause, considering that only System.Linq calls may represent an exit out of/entry in select clause.
There was a problem hiding this comment.
To put insideSelectClause = false; as it is in the default branch of switch statement.
There was a problem hiding this comment.
Ok, I've checked that my suggestion does not work.
|
Can you add a test to verify that assumption then?
|
Unless extending the LINQ query model with new custom clause extensions (things having a purpose similar to Anyway, I have added a test on key generation with constants in a where. |
* Fixes nhibernate#1330 (NH-3673) * Allows a workaround for nhibernate#1363 (NH-2500)
fredericDelaporte
force-pushed
the
NH-3673
branch
from
d2f6f3e to
ee6e31a
Compare
|
If #1542 gets merged before this one, this one would be obsoleted and in need of being closed. |
|
I have removed the |
There was a problem hiding this comment.
This works by adding
GetHashCodeto the key.
This is not guaranteed unique. This will cause the bug to still sometimes occurs, but in a evading, very hard to diagnose way. For me, with this "fix", we should still override ToString for being safe. But users will have a very hard time discovering they need this, because the trouble will most likely never occurs during testing, will not be reproductible, and will bug their application occasionally, causing it to be unreliable due to this design in NHibernate.
I was thinking of using something like this but that would over-complicate the code. (And that would introduce a counter overflow possibility for long running applications, either blowing away or taking a (very tiny) risk of getting the bug back. Well, with a long counter, it would takes hundreds of millions years to occurs, assuming the app is not able of running on average more than one query per millisecond ; so with a long we could disregard this overflow risk.)
| { | ||
| _string.Append(expression.Value); | ||
| _string.Append(value).Append("#").Append(value.GetHashCode()).Append(","); |
There was a problem hiding this comment.
I have a hard time conceiving a valid query having objects instances instead of primitive types in a list/array. Are you sure this is useful to change the enumerable case too?
|
Obsoleted by #1544. |
Includes #1539 commit.(Now merged and this PR is rebased.)