I took a look at the code and didn't see anything that would introduce
anything worse than potential performance issues. What I didn't understand
was why that particular set of method calls was selected. For example:
1. How is Select related to First/Single? Select doesn't force query
evaluation, but the other two do. Are we just assuming for simplicity that
the Select will be in memory?
2. What about calls like Aggregate or Last? I imagine we just punt on
those since the database query doesn't support them? I didn't check the
whole list.
3. While I'm not sure about the current implementation, what about selects
that happen in the database? This would at least be pertinent for
subqueries (which we don't support now anyways).
It seems that what you'd really want to say is "will this constant be used
in the database query?" Unfortunately that may be a hard question to answer.
I'm also curious what thoughts you had about the other solution posted on
the issue.
To make a long story short, I don't think that the changes would have a
significant negative effect and they certainly solve a current pain point.
Better to be correct and slow than fast and wrong. :)
Patrick Earl
On Tue, Jan 24, 2012 at 10:04 AM, Oren Eini (Ayende Rahien) <
[email protected]> wrote:
> Hi guys,
> I am trying to figure out something nasty in the linq provider.
>
> Take a look at the following queries:
>
> this.count = 1;
>> var foos1 = session.Query<Foo>()
>> .Where(x => x.Name == "Banana")
>> .Select(x => new
>> {
>> x.Name,
>> count,
>> User = "abc"
>> }).First();
>> this.count = 2;
>> var foos2 = session.Query<Foo>()
>> .Where(x => x.Name == "Egg")
>> .Select(x => new
>> {
>> x.Name,
>> count,
>> User = "def"
>> }).First();
>
>
>
> Right now, because we are caching the HQL plans, we are also caching the
> _constant values_.
> That is, The result of foos2 would be count = 1 (and not 2) and User =
> "abc" (and not "def")
>
> I pushed a failing test here:
> https://github.com/nhibernate/nhibernate-core/tree/nh-2500
>
> I am pretty sure that the actual reason for the error is that we are
> caching the lambdas. In other words, the HQL Query Plan is reusing the
> first lambda, instead of invoking the second one.
> I have modified the way we are caching query plans to take that into
> account. The way I do that is to check whatever we are inside a select
> clause, and if we are, to treat the constants there are real constants, and
> not as parameter constants for the purpose of generating the query plan key.
> All tests are passing, but I would still like to have a second pair of
> eyes on that.
>
