Bug fix guideance

[Jacob Levitt]

Would some of the more experienced NHB devs take a look at my comments on this bug and give me their opinions on how to proceed?

https://nhibernate.jira.com/browse/NH-3634

Thank you!

-Jake

[Oskar Berggren]

Traveling so don't have JIRA login right now, just gonna comment here instead.

From the looks of the sample in the issue report, it looks as if NH assumes that component equality means value equality of individual properties when converting to SQL. I think there might be precedent for this in linq2sql, and also by merit of being useful behavior in many cases.

So I think it's ok, and in that case it should really use the proper operator for null comparisons.

/Oskar

--

---
You received this message because you are subscribed to the Google Groups "nhibernate-development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-develo...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Gunnar Liljas]

But throwing on other comparisons might be a good idea.

/G

[Jacob Levitt]

Yeah I see your point - in some cases it is useful. The issue is that NHB tries to map equality to all of the component's properties. However, in some cases it needs change from equality to "is null". This still seems a bit weird since there's a separate Is.Null Restriction which the user didn't specify. Also, from debugging it doesn't seem like the Walker actually has access to the value being compared to (it just generates '?'s which get filled in later), so it doesn't seem like it has enough information to determine when to use equality and when to use "is null". Should I try to figure out a way to give the Walker more context about the actual values we're comparing to? It will need to know that that one of the component's properties is null in order to change the Eq constraint to a IsNull constraint.

Let me know if this isn't clear or I can provide more detail.

Thanks for the response!

-Jake

To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-development+unsub...@googlegroups.com.

[Jacob Levitt]

After looking at this more closely, it does seem like the context is there to make the decision. It still seems tricky though. It looks as though the SimpleExpression class will have to do something like:

if(paramValue==null&&Operation==" = ")
{
 //replace operation with "is null" and remove parameter from generated Sql
}
else
{
  //do existing logic
}

This doesn't feel right to me. Maybe I'm missing a better place to do this logic? If I add this in here, it seems that I will have to add other logic that affects the Loader, so it doesn't try to substitute parameters that are no longer there.

Any help is appreciated... I'm very new to NHB development!

-Jake

[Oskar Berggren]

Den 29 dec 2014 21:34 skrev "Jacob Levitt" <jle...@vt.edu>:
>
> Yeah I see your point - in some cases it is useful. The issue is that NHB tries to map equality to all of the component's properties. However, in some cases it needs change from equality to "is null". This still seems a bit weird since there's a separate Is.Null Restriction which the user didn't specify.

I wouldn't worry about that too much, but just consider the is null an implementation detail of SQL. Linq for instance has no separate null comparison operator, and in general it seems to work quite well to use C# semantics for null comparisons.

> Also, from debugging it doesn't seem like the Walker actually has access to the value being compared to (it just generates '?'s which get filled in later), so it doesn't seem like it has enough information to determine when to use equality and when to use "is null". Should I try to figure out a way to give the Walker more context about the actual values we're comparing to? It will need to know that that one of the component's properties is null in order to change the Eq constraint to a IsNull constraint.
>
> Let me know if this isn't clear or I can provide more detail.
>
> Thanks for the response!
>
> -Jake
>
>
> On Sunday, December 28, 2014 8:14:06 PM UTC-5, Oskar Berggren wrote:
>>
>> Traveling so don't have JIRA login right now, just gonna comment here instead.
>>
>> From the looks of the sample in the issue report, it looks as if NH assumes that component equality means value equality of individual properties when converting to SQL. I think there might be precedent for this in linq2sql, and also by merit of being useful behavior in many cases.
>>
>> So I think it's ok, and in that case it should really use the proper operator for null comparisons.
>>
>> /Oskar
>>
>> Den 28 dec 2014 18:17 skrev "Jacob Levitt" <jle...@vt.edu>:
>>
>>> Would some of the more experienced NHB devs take a look at my comments on this bug and give me their opinions on how to proceed?
>>>
>>> https://nhibernate.jira.com/browse/NH-3634
>>>
>>> Thank you!
>>> -Jake
>>>
>>> --
>>>
>>> ---
>>> You received this message because you are subscribed to the Google Groups "nhibernate-development" group.

>>> To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-develo...@googlegroups.com.

>>> For more options, visit https://groups.google.com/d/optout.
>

> --
>
> ---
> You received this message because you are subscribed to the Google Groups "nhibernate-development" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-develo...@googlegroups.com.