Allow complex boolean expressions in projection

Apply nullsemantics to projection
dotnet · Jan 11, 2016 · f1dd1ba · f1dd1ba
1 parent a47422f
commit f1dd1ba
Show file tree

Hide file tree

Showing 9 changed files with 967 additions and 838 deletions.
diff --git a/src/EntityFramework.MicrosoftSqlServer/Query/Sql/Internal/SqlServerQuerySqlGenerator.cs b/src/EntityFramework.MicrosoftSqlServer/Query/Sql/Internal/SqlServerQuerySqlGenerator.cs
@@ -102,9 +102,12 @@ public override Expression VisitSqlFunction(SqlFunctionExpression sqlFunctionExp
 
         private class ProjectionComparisonTransformingVisitor : RelinqExpressionVisitor
         {
+            private bool _insideConditionalTest = false;
+
             protected override Expression VisitUnary(UnaryExpression node)
             {
-                if (node.NodeType == ExpressionType.Not
+                if (!_insideConditionalTest
+                    && node.NodeType == ExpressionType.Not
                     && node.Operand is AliasExpression)
                 {
                     return Expression.Condition(
@@ -118,8 +121,10 @@ protected override Expression VisitUnary(UnaryExpression node)
 
             protected override Expression VisitBinary(BinaryExpression node)
             {
-                if (node.IsComparisonOperation())
-                {
+                if (!_insideConditionalTest
+                    && (node.IsComparisonOperation()
+                        || node.IsLogicalOperation()))
+                    {
                     return Expression.Condition(
                         node,
                         Expression.Constant(true, typeof(bool)),
@@ -132,7 +137,9 @@ protected override Expression VisitBinary(BinaryExpression node)
 
             protected override Expression VisitConditional(ConditionalExpression node)
             {
+                _insideConditionalTest = true;
                 var test = Visit(node.Test);
+                _insideConditionalTest = false;
                 if (test is AliasExpression)
                 {
                     return Expression.Condition(
@@ -149,7 +156,9 @@ protected override Expression VisitConditional(ConditionalExpression node)
                         Visit(node.IfTrue),
                         Visit(node.IfFalse));
                 }
-                return base.VisitConditional(node);
+                return Expression.Condition(test,
+                    Visit(node.IfTrue),
+                    Visit(node.IfFalse));
             }
         }
 

diff --git a/src/EntityFramework.Relational/Query/Sql/DefaultQuerySqlGenerator.cs b/src/EntityFramework.Relational/Query/Sql/DefaultQuerySqlGenerator.cs
@@ -174,22 +174,7 @@ public virtual Expression VisitSelect(SelectExpression selectExpression)
                 }
                 else
                 {
-                    var predicate
-                        = new NullComparisonTransformingVisitor(_parametersValues)
-                            .Visit(selectExpression.Predicate);
-
-                    var relationalNullsOptimizedExpandingVisitor = new RelationalNullsOptimizedExpandingVisitor();
-                    var newPredicate = relationalNullsOptimizedExpandingVisitor.Visit(predicate);
-
-                    predicate
-                        = relationalNullsOptimizedExpandingVisitor.IsOptimalExpansion
-                            ? newPredicate
-                            : new RelationalNullsExpandingVisitor().Visit(predicate);
-
-                    predicate = new PredicateNegationExpressionOptimizer().Visit(predicate);
-                    predicate = new ReducingExpressionVisitor().Visit(predicate);
-
-                    Visit(predicate);
+                    Visit(ApplyNullSemantics(selectExpression.Predicate));
 
                     if (selectExpression.Predicate is ParameterExpression
                         || selectExpression.Predicate.IsAliasWithColumnExpression()
@@ -227,13 +212,31 @@ var predicate
             return selectExpression;
         }
 
-        protected virtual void VisitProjection([NotNull] IReadOnlyList<Expression> projections)
+        private Expression ApplyNullSemantics(Expression expression)
         {
-            var nullComparisonTransformer = new NullComparisonTransformingVisitor(_parametersValues);
+            var newExpression
+                        = new NullComparisonTransformingVisitor(_parametersValues)
+                            .Visit(expression);
+
+            var relationalNullsOptimizedExpandingVisitor = new RelationalNullsOptimizedExpandingVisitor();
+            var optimizedExpression = relationalNullsOptimizedExpandingVisitor.Visit(newExpression);
+
+            newExpression
+                = relationalNullsOptimizedExpandingVisitor.IsOptimalExpansion
+                    ? optimizedExpression
+                    : new RelationalNullsExpandingVisitor().Visit(newExpression);
 
-            VisitJoin(projections.Select(e => nullComparisonTransformer.Visit(e)).ToList());
+            newExpression = new PredicateNegationExpressionOptimizer().Visit(newExpression);
+            newExpression = new ReducingExpressionVisitor().Visit(newExpression);
+
+            return newExpression;
         }
 
+        protected virtual void VisitProjection([NotNull] IReadOnlyList<Expression> projections) => VisitJoin(
+            projections
+                .Select(ApplyNullSemantics)
+                .ToList());
+
         protected virtual void GenerateOrderBy([NotNull] IReadOnlyList<Ordering> orderings)
         {
             _relationalCommandBuilder.Append("ORDER BY ");

diff --git a/test/EntityFramework.Core.FunctionalTests/GearsOfWarQueryTestBase.cs b/test/EntityFramework.Core.FunctionalTests/GearsOfWarQueryTestBase.cs
@@ -522,23 +522,23 @@ public virtual void Select_comparison_with_null()
             AmmunitionType? ammunitionType = AmmunitionType.Cartridge;
             using (var context = CreateContext())
             {
-                var cartidgeWeapons = context.Weapons
+                var cartridgeWeapons = context.Weapons
                     .Where(w => w.AmmunitionType == ammunitionType)
                     .Select(w => new { w.Id, Cartidge = w.AmmunitionType == ammunitionType })
                     .ToList();
 
-                Assert.True(cartidgeWeapons.All(t => t.Cartidge == true));
+                Assert.True(cartridgeWeapons.All(t => t.Cartidge == true));
             }
 
             ammunitionType = null;
             using (var context = CreateContext())
             {
-                var cartidgeWeapons = context.Weapons
+                var cartridgeWeapons = context.Weapons
                     .Where(w => w.AmmunitionType == ammunitionType)
                     .Select(w => new { w.Id, Cartidge = w.AmmunitionType == ammunitionType })
                     .ToList();
 
-                Assert.True(cartidgeWeapons.All(t => t.Cartidge == true));
+                Assert.True(cartridgeWeapons.All(t => t.Cartidge == true));
             }
         }
 
@@ -568,6 +568,73 @@ public virtual void Select_ternary_operation_with_inverted_boolean()
             }
         }
 
+        [ConditionalFact]
+        public virtual void Select_ternary_operation_with_has_value_not_null()
+        {
+            using (var context = CreateContext())
+            {
+                var cartridgeWeapons = context.Weapons
+                    .Where(w => w.AmmunitionType.HasValue && w.AmmunitionType == AmmunitionType.Cartridge)
+                    .Select(w => new { w.Id, IsCartidge = w.AmmunitionType.HasValue && w.AmmunitionType.Value == AmmunitionType.Cartridge ? "Yes" : "No"})
+                    .ToList();
+
+                Assert.All(cartridgeWeapons,
+                    t => Assert.Equal("Yes", t.IsCartidge));
+            }
+        }
+
+        [ConditionalFact]
+        public virtual void Select_ternary_operation_multiple_conditions()
+        {
+            using (var context = CreateContext())
+            {
+                var cartridgeWeapons = context.Weapons
+                    .Select(w => new { w.Id, IsCartidge = w.AmmunitionType == AmmunitionType.Shell && w.SynergyWithId == 1 ? "Yes" : "No" })
+                    .ToList();
+
+                Assert.Equal(9 , cartridgeWeapons.Count(w => w.IsCartidge == "No"));
+            }
+        }
+
+        [ConditionalFact]
+        public virtual void Select_ternary_operation_multiple_conditions_2()
+        {
+            using (var context = CreateContext())
+            {
+                var cartridgeWeapons = context.Weapons
+                    .Select(w => new { w.Id, IsCartidge = !w.IsAutomatic && w.SynergyWithId == 1 ? "Yes" : "No" })
+                    .ToList();
+
+                Assert.Equal(9, cartridgeWeapons.Count(w => w.IsCartidge == "No"));
+            }
+        }
+
+        [ConditionalFact]
+        public virtual void Select_multiple_conditions()
+        {
+            using (var context = CreateContext())
+            {
+                var cartridgeWeapons = context.Weapons
+                    .Select(w => new { w.Id, IsCartidge = !w.IsAutomatic && w.SynergyWithId == 1})
+                    .ToList();
+
+                Assert.Equal(9, cartridgeWeapons.Count(w => !w.IsCartidge));
+            }
+        }
+
+        [ConditionalFact]
+        public virtual void Select_nested_ternary_operations()
+        {
+            using (var context = CreateContext())
+            {
+                var cartridgeWeapons = context.Weapons
+                    .Select(w => new { w.Id, IsManualCartidge = !w.IsAutomatic ? w.AmmunitionType == AmmunitionType.Cartridge ? "ManualCartridge" : "Manual" : "Auto" })
+                    .ToList();
+
+                Assert.Equal(2, cartridgeWeapons.Count(w => w.IsManualCartidge == "ManualCartridge"));
+            }
+        }
+
         [ConditionalFact]
         public virtual void Select_Where_Navigation()
         {