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
>
