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. |