Okay, I've already decided my change doesn't look sane, but now I
don't know why it fixed the test. Back to the drawing board. :O
Patrick Earl
On Sun, Apr 17, 2011 at 4:44 PM, Patrick Earl <[email protected]> wrote:
> I improved some of the tests and found a bug in the implementation
> when I enabled a previously disabled test. If you could tell me if my
> constant visitor looks sane, that would be nice.
>
> I agree that your suggestion sounds like the right thing to do for the
> non-boolean operators.
>
> Patrick Earl
>
> On Sun, Apr 17, 2011 at 2:00 PM, Harald Mueller <[email protected]>
> wrote:
>> A quick look into the code makes me think that lines 193 to 201 of
>> WhereJoinDetector are a copy-paste error: They should certainly NOT be
>>
>> else // +, * etc.
>> {
>> // Case (j)
>> _collectedPathMemberExpressionsInExpression = new HashSet<string>();
>>
>> baseResult = base.VisitBinaryExpression(expression);
>>
>> _memberExpressionMapping =
>> FixedMapping(_collectedPathMemberExpressionsInExpression, TNF);
>> }
>>
>> but PROBABLY
>>
>> else // +, * etc.
>> {
>> // Case (j)
>> baseResult = base.VisitBinaryExpression(expression);
>> }
>>
>> However, I did not even try to compile it, much less test it - it just seems
>> "obvious", doesn't it? - after all, for such arbitrary sub-expressions,
>> we(I) should just collect the information in
>> _collectedPathMemberExpressionsInExpression up to the lowest comparison
>> operator!
>>
>> Regards
>> Harald
>>
>>
>> -------- Original-Nachricht --------
>>> Datum: Sun, 17 Apr 2011 11:33:24 -0600
>>> Von: Patrick Earl <[email protected]>
>>> An: [email protected]
>>> Betreff: Re: RE: [nhibernate-development] NH-2583 - Query with || operator
>>> and navigations (many-to-one) creates wrong joins
>>
>>> Hi Harald.
>>>
>>> After did a sanity check on the patch and then committed it. I'm in
>>> the middle of a deeper analysis and I found something I wanted to
>>> check with you about.
>>>
>>> It seems that the _collectedPathMemberExpressionsInExpression and
>>> _memberExpressionMapping fields are troublesome in places. For
>>> example, when doing (A+(B+C)) it seems to drop the members collected
>>> in A when it starts parsing the right side. In looking at the
>>> _memberExpressionMapping, other places it combines the inner results
>>> into a parent _memberExpressionMapping, but for the +, there is no
>>> such collection. The _collectedPathMemberExpressionsInExpression
>>> seems to have similar issues, though I didn't analyze it in depth.
>>>
>>> Could you look into this?
>>>
>>> Appreciate your help.
>>>
>>> Patrick Earl
>>
>> --
>> GMX DSL Doppel-Flat ab 19,99 Euro/mtl.! Jetzt mit
>> gratis Handy-Flat! http://portal.gmx.net/de/go/dsl
>>
>
