|
From: <pa...@us...> - 2011-04-18 17:25:07
|
Revision: 5717
http://nhibernate.svn.sourceforge.net/nhibernate/?rev=5717&view=rev
Author: patearl
Date: 2011-04-18 17:25:00 +0000 (Mon, 18 Apr 2011)
Log Message:
-----------
Linq: More changes related to NH-2583. Use a stack to help eliminate member variable leakage. Handle stand-alone boolean member and method call expressions. Eliminate an earlier invalid attempted fix.
Modified Paths:
--------------
trunk/nhibernate/src/NHibernate/Linq/Visitors/WhereJoinDetector.cs
Modified: trunk/nhibernate/src/NHibernate/Linq/Visitors/WhereJoinDetector.cs
===================================================================
--- trunk/nhibernate/src/NHibernate/Linq/Visitors/WhereJoinDetector.cs 2011-04-17 22:28:45 UTC (rev 5716)
+++ trunk/nhibernate/src/NHibernate/Linq/Visitors/WhereJoinDetector.cs 2011-04-18 17:25:00 UTC (rev 5717)
@@ -1,3 +1,4 @@
+using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using NHibernate.Linq.ReWriters;
@@ -102,10 +103,10 @@
// The following is used for all *condition* traversal (but not *expressions* that are not conditions).
// This is the "mapping" described in the text at NH-2583.
- private Dictionary<string, int> _memberExpressionMapping = new Dictionary<string, int>();
+ private Stack<Dictionary<string, int>> _memberExpressionMappings = new Stack<Dictionary<string, int>>();
// The following two are used for member expressions traversal.
- private HashSet<string> _collectedPathMemberExpressionsInExpression = new HashSet<string>();
+ private Stack<HashSet<string>> _collectedPathMemberExpressionsInExpressions = new Stack<HashSet<string>>();
private int _memberExpressionDepth = 0;
internal
@@ -122,16 +123,16 @@
if (expression.NodeType == ExpressionType.AndAlso && expression.Type == typeof(bool))
{
// Case (a) from the text at NH-2583.
+ _memberExpressionMappings.Push(new Dictionary<string, int>());
var newLeft = VisitExpression(expression.Left);
+ var leftMapping = _memberExpressionMappings.Pop();
- var leftMapping = _memberExpressionMapping;
-
+ _memberExpressionMappings.Push(new Dictionary<string, int>());
var newRight = VisitExpression(expression.Right);
+ var rightMapping = _memberExpressionMappings.Pop();
- var rightMapping = _memberExpressionMapping;
+ BinaryMapping(_memberExpressionMappings.Peek(), leftMapping, rightMapping, AND);
- _memberExpressionMapping = BinaryMapping(leftMapping, rightMapping, AND);
-
// The following is copy-pasted from Relinq's visitor, as I had to split the code above.
var newConversion = (LambdaExpression)VisitExpression(expression.Conversion);
if (newLeft != expression.Left || newRight != expression.Right || newConversion != expression.Conversion)
@@ -140,16 +141,16 @@
else if (expression.NodeType == ExpressionType.OrElse && expression.Type == typeof(bool))
{
// Case (b)
+ _memberExpressionMappings.Push(new Dictionary<string, int>());
var newLeft = VisitExpression(expression.Left);
+ var leftMapping = _memberExpressionMappings.Pop();
- var leftMapping = _memberExpressionMapping;
-
+ _memberExpressionMappings.Push(new Dictionary<string, int>());
var newRight = VisitExpression(expression.Right);
+ var rightMapping = _memberExpressionMappings.Pop();
- var rightMapping = _memberExpressionMapping;
+ BinaryMapping(_memberExpressionMappings.Peek(), leftMapping, rightMapping, OR);
- _memberExpressionMapping = BinaryMapping(leftMapping, rightMapping, OR);
-
// Again, the following is copy-pasted from Relinq's visitor, as I had to split the code above.
var newConversion = (LambdaExpression)VisitExpression(expression.Conversion);
if (newLeft != expression.Left || newRight != expression.Right || newConversion != expression.Conversion)
@@ -164,63 +165,60 @@
|| expression.NodeType == ExpressionType.GreaterThanOrEqual))
{
// Cases (e), (f).2, (g).2
- _collectedPathMemberExpressionsInExpression = new HashSet<string>();
+ _collectedPathMemberExpressionsInExpressions.Push(new HashSet<string>());
baseResult = base.VisitBinaryExpression(expression);
- _memberExpressionMapping = FixedMapping(_collectedPathMemberExpressionsInExpression, N);
+ FixedMapping(_memberExpressionMappings.Peek(), _collectedPathMemberExpressionsInExpressions.Pop(), N);
}
else if (expression.Type == typeof(bool)
&& expression.NodeType == ExpressionType.NotEqual)
{
// Case (h)
- _collectedPathMemberExpressionsInExpression = new HashSet<string>();
+ _collectedPathMemberExpressionsInExpressions.Push(new HashSet<string>());
baseResult = base.VisitBinaryExpression(expression);
- _memberExpressionMapping = FixedMapping(_collectedPathMemberExpressionsInExpression, F);
+ FixedMapping(_memberExpressionMappings.Peek(), _collectedPathMemberExpressionsInExpressions.Pop(), F);
}
else if (expression.Type == typeof(bool)
&& expression.NodeType == ExpressionType.Equal)
{
// Case (i)
- _collectedPathMemberExpressionsInExpression = new HashSet<string>();
+ _collectedPathMemberExpressionsInExpressions.Push(new HashSet<string>());
baseResult = base.VisitBinaryExpression(expression);
- _memberExpressionMapping = FixedMapping(_collectedPathMemberExpressionsInExpression, T);
+ FixedMapping(_memberExpressionMappings.Peek(), _collectedPathMemberExpressionsInExpressions.Pop(), T);
}
else // +, * etc.
{
// Case (j)
- _collectedPathMemberExpressionsInExpression = new HashSet<string>();
-
baseResult = base.VisitBinaryExpression(expression);
-
- _memberExpressionMapping = FixedMapping(_collectedPathMemberExpressionsInExpression, TNF);
}
return baseResult;
}
- private static Dictionary<string, int> FixedMapping(IEnumerable<string> collectedPathMemberExpressionsInExpression, int value)
+ private static void FixedMapping(Dictionary<string, int> resultMapping, IEnumerable<string> collectedPathMemberExpressionsInExpression, int value)
{
- var memberExpressionMapping = new Dictionary<string, int>();
foreach (var me in collectedPathMemberExpressionsInExpression)
{
- memberExpressionMapping.Add(me, value);
+ // This ORing behavior was added without much thought to fix NHibernate.Test.NHSpecificTest.NH2378.Fixture.ShortEntityCanBeQueryCorrectlyUsingLinqProvider.
+ if (resultMapping.ContainsKey(me))
+ resultMapping[me] |= value;
+ else
+ resultMapping.Add(me, value);
}
- return memberExpressionMapping;
}
- private static Dictionary<string, int> BinaryMapping(Dictionary<string, int> leftMapping, Dictionary<string, int> rightMapping, int[,] op)
+ private static void BinaryMapping(Dictionary<string, int> resultMapping, Dictionary<string, int> leftMapping, Dictionary<string, int> rightMapping, int[,] op)
{
- var result = new Dictionary<string, int>();
// Compute mapping for all member expressions in leftMapping. If the member expression is missing
// in rightMapping, use TNF as a "pessimistic approximation" instead (inside the ?: operator). See
// the text for an explanation of this.
foreach (var lhs in leftMapping)
{
- result.Add(lhs.Key, op[lhs.Value, rightMapping.ContainsKey(lhs.Key) ? rightMapping[lhs.Key] : TNF]);
+ resultMapping.Add(lhs.Key, op[lhs.Value, rightMapping.ContainsKey(lhs.Key) ? rightMapping[lhs.Key] : TNF]);
}
// Compute mapping for all member expressions *only* in rightMapping (we did the common ones above).
// Again, use TNF as pessimistic approximation to result of left subcondition.
@@ -228,10 +226,9 @@
{
if (!leftMapping.ContainsKey(rhs.Key))
{
- result[rhs.Key] = op[rhs.Value, TNF];
+ resultMapping[rhs.Key] = op[rhs.Value, TNF];
}
}
- return result;
}
private static bool IsNullConstantExpression(Expression expression)
@@ -255,14 +252,13 @@
if (expression.NodeType == ExpressionType.Not && expression.Type == typeof(bool))
{
// Case (c) from text at NH-2583.
+ _memberExpressionMappings.Push(new Dictionary<string, int>());
baseResult = VisitExpression(expression.Operand);
+ var opMapping = _memberExpressionMappings.Pop();
- var opMapping = _memberExpressionMapping;
- _memberExpressionMapping = new Dictionary<string, int>();
-
foreach (var m in opMapping)
{
- _memberExpressionMapping.Add(m.Key, NOT[m.Value]);
+ _memberExpressionMappings.Peek().Add(m.Key, NOT[m.Value]);
}
}
else
@@ -286,63 +282,98 @@
// return expression;
//}
- protected override Expression VisitMemberExpression(MemberExpression expression)
+ protected override Expression VisitMethodCallExpression(MethodCallExpression expression)
{
- ArgumentUtility.CheckNotNull("expression", expression);
+ // Similar logic to VisitMemberExpression, but for handling boolean method results instead of boolean members.
+ bool addOwnMemberExpressionMapping = false;
+ if (_memberExpressionDepth == 0 && _collectedPathMemberExpressionsInExpressions.Count == 0)
+ {
+ if (expression.Type != typeof(bool))
+ throw new AssertionFailure("Was expected a boolean member expression.");
+ addOwnMemberExpressionMapping = true;
+ _collectedPathMemberExpressionsInExpressions.Push(new HashSet<string>());
+ }
- Expression newExpression;
try
{
- _memberExpressionDepth++;
- newExpression = base.VisitExpression(expression.Expression);
+ return base.VisitMethodCallExpression(expression);
}
finally
{
- _memberExpressionDepth--;
+ if (addOwnMemberExpressionMapping)
+ {
+ // Same mapping as the not-null (in)equality clause in VisitBinaryExpression.
+ FixedMapping(_memberExpressionMappings.Peek(), _collectedPathMemberExpressionsInExpressions.Pop(), N);
+ }
}
- bool isEntity = _isEntityDecider.IsEntity(expression.Type);
-
- if (isEntity)
- {
- // See (h) why we do not check for _memberExpressionDepth here!
- _collectedPathMemberExpressionsInExpression.Add(ExpressionKeyVisitor.Visit(expression, null));
- }
+ }
- if (_memberExpressionDepth > 0 && isEntity)
+ protected override Expression VisitMemberExpression(MemberExpression expression)
+ {
+ ArgumentUtility.CheckNotNull("expression", expression);
+
+ // For the boolean part of expressions like: a => a.B.C == 1 && a.D.E
+ // There wouldn't have been a parent operator to collect paths for a.D.E.
+ // Below we add the same mapping as if we were doing a.D.E == true.
+ bool addOwnMemberExpressionMapping = false;
+ if (_memberExpressionDepth == 0 && _collectedPathMemberExpressionsInExpressions.Count == 0)
{
- return AddJoin(expression);
+ if (expression.Type != typeof(bool))
+ throw new AssertionFailure("Was expected a boolean member expression.");
+ addOwnMemberExpressionMapping = true;
+ _collectedPathMemberExpressionsInExpressions.Push(new HashSet<string>());
}
- else
- {
- if (newExpression != expression.Expression)
- return Expression.MakeMemberAccess(newExpression, expression.Member);
- return expression;
- }
- }
- protected override Expression VisitConstantExpression(ConstantExpression expression)
- {
- if (expression.Type == typeof(bool))
+ try
{
- if ((bool)expression.Value)
+ Expression newExpression;
+ try
{
- _memberExpressionMapping = FixedMapping(new string[0], T);
+ _memberExpressionDepth++;
+ newExpression = base.VisitExpression(expression.Expression);
}
+ finally
+ {
+ _memberExpressionDepth--;
+ }
+ bool isEntity = _isEntityDecider.IsEntity(expression.Type);
+
+ if (isEntity)
+ {
+ // See (h) why we do not check for _memberExpressionDepth here!
+ _collectedPathMemberExpressionsInExpressions.Peek().Add(ExpressionKeyVisitor.Visit(expression, null));
+ }
+
+ if (_memberExpressionDepth > 0 && isEntity)
+ {
+ return AddJoin(expression);
+ }
else
{
- _memberExpressionMapping = FixedMapping(new string[0], F);
+ if (newExpression != expression.Expression)
+ return Expression.MakeMemberAccess(newExpression, expression.Member);
+ return expression;
}
}
- return expression;
+ finally
+ {
+ if (addOwnMemberExpressionMapping)
+ {
+ // Same mapping as the not-null (in)equality clause in VisitBinaryExpression.
+ FixedMapping(_memberExpressionMappings.Peek(), _collectedPathMemberExpressionsInExpressions.Pop(), N);
+ }
+ }
}
internal static void Find(Expression expression, NameGenerator nameGenerator, IIsEntityDecider isEntityDecider, Dictionary<string, NhJoinClause> joins, Dictionary<MemberExpression, QuerySourceReferenceExpression> expressionMap)
{
WhereJoinDetector f = new WhereJoinDetector(nameGenerator, isEntityDecider, joins, expressionMap);
+ f._memberExpressionMappings.Push(new Dictionary<string, int>());
+
f.VisitExpression(expression);
- foreach (var mapping in f._memberExpressionMapping)
+ foreach (var mapping in f._memberExpressionMappings.Pop())
{
// If outer join can never produce true, we can safely inner join.
if ((mapping.Value & T) == 0)
@@ -351,6 +382,5 @@
}
}
}
-
}
}
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
|