diff --git a/src/EFCore.Relational/Query/ISqlExpressionFactory.cs b/src/EFCore.Relational/Query/ISqlExpressionFactory.cs index 8e0cf8114bf..6d87975b472 100644 --- a/src/EFCore.Relational/Query/ISqlExpressionFactory.cs +++ b/src/EFCore.Relational/Query/ISqlExpressionFactory.cs @@ -57,12 +57,14 @@ public interface ISqlExpressionFactory /// The left operand of binary operation. /// The right operand of binary operation. /// A type mapping to be assigned to the created expression. + /// An optional expression that can be re-used if it matches the new expression. /// A with the given arguments. SqlExpression? MakeBinary( ExpressionType operatorType, SqlExpression left, SqlExpression right, - RelationalTypeMapping? typeMapping); + RelationalTypeMapping? typeMapping, + SqlExpression? existingExpr = null); // Comparison /// diff --git a/src/EFCore.Relational/Query/SqlExpressionFactory.cs b/src/EFCore.Relational/Query/SqlExpressionFactory.cs index 3e195b157e3..8e3a5672aec 100644 --- a/src/EFCore.Relational/Query/SqlExpressionFactory.cs +++ b/src/EFCore.Relational/Query/SqlExpressionFactory.cs @@ -395,8 +395,17 @@ private SqlExpression ApplyTypeMappingOnJsonScalar( ExpressionType operatorType, SqlExpression left, SqlExpression right, - RelationalTypeMapping? typeMapping) + RelationalTypeMapping? typeMapping, + SqlExpression? existingExpr = null) { + switch (operatorType) + { + case ExpressionType.AndAlso: + return ApplyTypeMapping(AndAlso(left, right, existingExpr), typeMapping); + case ExpressionType.OrElse: + return ApplyTypeMapping(OrElse(left, right, existingExpr), typeMapping); + } + if (!SqlBinaryExpression.IsValidOperator(operatorType)) { return null; @@ -411,8 +420,6 @@ private SqlExpression ApplyTypeMappingOnJsonScalar( case ExpressionType.LessThan: case ExpressionType.LessThanOrEqual: case ExpressionType.NotEqual: - case ExpressionType.AndAlso: - case ExpressionType.OrElse: returnType = typeof(bool); break; } @@ -449,10 +456,87 @@ public virtual SqlExpression LessThanOrEqual(SqlExpression left, SqlExpression r public virtual SqlExpression AndAlso(SqlExpression left, SqlExpression right) => MakeBinary(ExpressionType.AndAlso, left, right, null)!; + private SqlExpression AndAlso(SqlExpression left, SqlExpression right, SqlExpression? existingExpr) + { + // false && x -> false + // x && true -> x + // x && x -> x + if (left is SqlConstantExpression { Value: false } + || right is SqlConstantExpression { Value: true } + || left.Equals(right)) + { + return left; + } + // true && x -> x + // x && false -> false + else if (left is SqlConstantExpression { Value: true } || right is SqlConstantExpression { Value: false }) + { + return right; + } + // x is null && x is not null -> false + // x is not null && x is null -> false + else if (left is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } leftUnary + && right is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } rightUnary + && leftUnary.Operand.Equals(rightUnary.Operand)) + { + // the case in which left and right are the same expression is handled above + return Constant(false); + } + else if (existingExpr is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } binaryExpr + && left == binaryExpr.Left + && right == binaryExpr.Right) + { + return existingExpr; + } + else + { + return new SqlBinaryExpression(ExpressionType.AndAlso, left, right, typeof(bool), null); + } + } + /// public virtual SqlExpression OrElse(SqlExpression left, SqlExpression right) => MakeBinary(ExpressionType.OrElse, left, right, null)!; + private SqlExpression OrElse(SqlExpression left, SqlExpression right, SqlExpression? existingExpr = null) + { + // true || x -> true + // x || false -> x + // x || x -> x + if (left is SqlConstantExpression { Value: true } + || right is SqlConstantExpression { Value: false } + || left.Equals(right)) + { + return left; + } + // false || x -> x + // x || true -> true + else if (left is SqlConstantExpression { Value: false } + || right is SqlConstantExpression { Value: true }) + { + return right; + } + // x is null || x is not null -> true + // x is not null || x is null -> true + else if (left is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } leftUnary + && right is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } rightUnary + && leftUnary.Operand.Equals(rightUnary.Operand)) + { + // the case in which left and right are the same expression is handled above + return Constant(true); + } + else if (existingExpr is SqlBinaryExpression { OperatorType: ExpressionType.OrElse } binaryExpr + && left == binaryExpr.Left + && right == binaryExpr.Right) + { + return existingExpr; + } + else + { + return new SqlBinaryExpression(ExpressionType.OrElse, left, right, typeof(bool), null); + } + } + /// public virtual SqlExpression Add(SqlExpression left, SqlExpression right, RelationalTypeMapping? typeMapping = null) => MakeBinary(ExpressionType.Add, left, right, typeMapping)!; diff --git a/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs b/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs index 4b2b44773c1..634429ff98e 100644 --- a/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs +++ b/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs @@ -1358,8 +1358,14 @@ or ExpressionType.LessThan } return result is SqlBinaryExpression sqlBinaryResult - && sqlBinaryExpression.OperatorType is ExpressionType.AndAlso or ExpressionType.OrElse - ? SimplifyLogicalSqlBinaryExpression(sqlBinaryResult) + && sqlBinaryResult.OperatorType is ExpressionType.AndAlso or ExpressionType.OrElse + ? _sqlExpressionFactory.MakeBinary( // invoke MakeBinary simplifications + sqlBinaryResult.OperatorType, + sqlBinaryResult.Left, + sqlBinaryResult.Right, + sqlBinaryResult.TypeMapping, + sqlBinaryResult + )! : result; SqlExpression AddNullConcatenationProtection(SqlExpression argument, RelationalTypeMapping typeMapping) @@ -1796,26 +1802,16 @@ private SqlExpression RewriteNullSemantics( { nullable = leftNullable || rightNullable; - return SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.OrElse( - body, - SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull)))); + return _sqlExpressionFactory.OrElse(body, _sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull)); } // doing a full null semantics rewrite - removing all nulls from truth table nullable = false; // (a == b && (a != null && b != null)) || (a == null && b == null) - body = SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.OrElse( - SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.AndAlso( - body, - SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.AndAlso(leftIsNotNull, rightIsNotNull)))), - SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull)))); + body = _sqlExpressionFactory.OrElse( + _sqlExpressionFactory.AndAlso(body, _sqlExpressionFactory.AndAlso(leftIsNotNull, rightIsNotNull)), + _sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull)); if (sqlBinaryExpression.OperatorType == ExpressionType.NotEqual) { @@ -1826,65 +1822,6 @@ private SqlExpression RewriteNullSemantics( return body; } - private SqlExpression SimplifyLogicalSqlBinaryExpression(SqlExpression expression) - { - if (expression is not SqlBinaryExpression sqlBinaryExpression) - { - return expression; - } - - if (sqlBinaryExpression is - { - Left: SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } leftUnary, - Right: SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } rightUnary - } - && leftUnary.Operand.Equals(rightUnary.Operand)) - { - // a is null || a is null -> a is null - // a is not null || a is not null -> a is not null - // a is null && a is null -> a is null - // a is not null && a is not null -> a is not null - // a is null || a is not null -> true - // a is null && a is not null -> false - return leftUnary.OperatorType == rightUnary.OperatorType - ? leftUnary - : _sqlExpressionFactory.Constant( - sqlBinaryExpression.OperatorType == ExpressionType.OrElse, sqlBinaryExpression.TypeMapping); - } - - // true && a -> a - // true || a -> true - // false && a -> false - // false || a -> a - if (sqlBinaryExpression.Left is SqlConstantExpression { Value: bool leftBoolValue } newLeftConstant) - { - return sqlBinaryExpression.OperatorType == ExpressionType.AndAlso - ? leftBoolValue - ? sqlBinaryExpression.Right - : newLeftConstant - : leftBoolValue - ? newLeftConstant - : sqlBinaryExpression.Right; - } - - if (sqlBinaryExpression.Right is SqlConstantExpression { Value: bool rightBoolValue } newRightConstant) - { - // a && true -> a - // a || true -> true - // a && false -> false - // a || false -> a - return sqlBinaryExpression.OperatorType == ExpressionType.AndAlso - ? rightBoolValue - ? sqlBinaryExpression.Left - : newRightConstant - : rightBoolValue - ? newRightConstant - : sqlBinaryExpression.Left; - } - - return sqlBinaryExpression; - } - /// /// Attempts to simplify a unary not operation on a non-nullable operand. /// @@ -1938,14 +1875,13 @@ protected virtual SqlExpression OptimizeNonNullableNotExpression(SqlExpression e var left = OptimizeNonNullableNotExpression(_sqlExpressionFactory.Not(sqlBinaryOperand.Left)); var right = OptimizeNonNullableNotExpression(_sqlExpressionFactory.Not(sqlBinaryOperand.Right)); - return SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.MakeBinary( - sqlBinaryOperand.OperatorType == ExpressionType.AndAlso - ? ExpressionType.OrElse - : ExpressionType.AndAlso, - left, - right, - sqlBinaryOperand.TypeMapping)!); + return _sqlExpressionFactory.MakeBinary( + sqlBinaryOperand.OperatorType == ExpressionType.AndAlso + ? ExpressionType.OrElse + : ExpressionType.AndAlso, + left, + right, + sqlBinaryOperand.TypeMapping)!; } // use equality where possible @@ -2266,14 +2202,13 @@ private SqlExpression ProcessNullNotNull(SqlExpression sqlExpression, bool opera sqlUnaryExpression.TypeMapping)!, operandNullable); - return SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.MakeBinary( - sqlUnaryExpression.OperatorType == ExpressionType.Equal - ? ExpressionType.OrElse - : ExpressionType.AndAlso, - left, - right, - sqlUnaryExpression.TypeMapping)!); + return _sqlExpressionFactory.MakeBinary( + sqlUnaryExpression.OperatorType == ExpressionType.Equal + ? ExpressionType.OrElse + : ExpressionType.AndAlso, + left, + right, + sqlUnaryExpression.TypeMapping)!; } case SqlFunctionExpression sqlFunctionExpression: @@ -2295,14 +2230,13 @@ private SqlExpression ProcessNullNotNull(SqlExpression sqlExpression, bool opera sqlUnaryExpression.TypeMapping)!, operandNullable)) .Aggregate( - (l, r) => SimplifyLogicalSqlBinaryExpression( - _sqlExpressionFactory.MakeBinary( - sqlUnaryExpression.OperatorType == ExpressionType.Equal - ? ExpressionType.AndAlso - : ExpressionType.OrElse, - l, - r, - sqlUnaryExpression.TypeMapping)!)); + (l, r) => _sqlExpressionFactory.MakeBinary( + sqlUnaryExpression.OperatorType == ExpressionType.Equal + ? ExpressionType.AndAlso + : ExpressionType.OrElse, + l, + r, + sqlUnaryExpression.TypeMapping)!); } if (!sqlFunctionExpression.IsNullable) @@ -2348,10 +2282,9 @@ private SqlExpression ProcessNullNotNull(SqlExpression sqlExpression, bool opera sqlUnaryExpression.TypeMapping)!, operandNullable)) .Aggregate( - (r, e) => SimplifyLogicalSqlBinaryExpression( - sqlUnaryExpression.OperatorType == ExpressionType.Equal - ? _sqlExpressionFactory.OrElse(r, e) - : _sqlExpressionFactory.AndAlso(r, e))); + (r, e) => sqlUnaryExpression.OperatorType == ExpressionType.Equal + ? _sqlExpressionFactory.OrElse(r, e) + : _sqlExpressionFactory.AndAlso(r, e)); return result; } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs index e970afe5674..bab76f79777 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs @@ -7156,7 +7156,7 @@ public override async Task Query_reusing_parameter_doesnt_declare_duplicate_para FROM ( SELECT DISTINCT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank] FROM [Gears] AS [g] - WHERE [g].[Nickname] <> @__prm_Inner_Nickname_0 AND [g].[Nickname] <> @__prm_Inner_Nickname_0 + WHERE [g].[Nickname] <> @__prm_Inner_Nickname_0 ) AS [g0] ORDER BY [g0].[FullName] """); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs index e9341157ff7..a060e8d211a 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs @@ -187,7 +187,7 @@ public override async Task Join_composite_key(bool async) """ SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region], [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate] FROM [Customers] AS [c] -INNER JOIN [Orders] AS [o] ON [c].[CustomerID] = [o].[CustomerID] AND [c].[CustomerID] = [o].[CustomerID] +INNER JOIN [Orders] AS [o] ON [c].[CustomerID] = [o].[CustomerID] WHERE [c].[CustomerID] LIKE N'F%' """); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs index ca7002f78cd..047d9c29e53 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindMiscellaneousQuerySqlServerTest.cs @@ -5185,7 +5185,7 @@ FROM [Customers] AS [c] WHERE ( SELECT COUNT(*) FROM [Orders] AS [o] - WHERE [c].[CustomerID] = [o].[CustomerID] AND [c].[CustomerID] = [o].[CustomerID]) > 0 + WHERE [c].[CustomerID] = [o].[CustomerID]) > 0 """); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs index af2b5cb528d..05d552823cb 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs @@ -9528,7 +9528,7 @@ UNION ALL SELECT [o].[Nickname], [o].[SquadId], [o].[AssignedCityName], [o].[CityOfBirthName], [o].[FullName], [o].[HasSoulPatch], [o].[LeaderNickname], [o].[LeaderSquadId], [o].[Rank], N'Officer' AS [Discriminator] FROM [Officers] AS [o] ) AS [u] - WHERE [u].[Nickname] <> @__prm_Inner_Nickname_0 AND [u].[Nickname] <> @__prm_Inner_Nickname_0 + WHERE [u].[Nickname] <> @__prm_Inner_Nickname_0 ) AS [u0] ORDER BY [u0].[FullName] """); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs index 6e857565770..6d70e3ad9ed 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs @@ -8137,7 +8137,7 @@ WHEN [o].[Nickname] IS NOT NULL THEN N'Officer' END AS [Discriminator] FROM [Gears] AS [g] LEFT JOIN [Officers] AS [o] ON [g].[Nickname] = [o].[Nickname] AND [g].[SquadId] = [o].[SquadId] - WHERE [g].[Nickname] <> @__prm_Inner_Nickname_0 AND [g].[Nickname] <> @__prm_Inner_Nickname_0 + WHERE [g].[Nickname] <> @__prm_Inner_Nickname_0 ) AS [s] ORDER BY [s].[FullName] """); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs index 7ee56fa0222..d493546fadd 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs @@ -2943,7 +2943,7 @@ public override async Task Query_reusing_parameter_doesnt_declare_duplicate_para FROM ( SELECT DISTINCT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[PeriodEnd], [g].[PeriodStart], [g].[Rank] FROM [Gears] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [g] - WHERE [g].[Nickname] <> @__prm_Inner_Nickname_0 AND [g].[Nickname] <> @__prm_Inner_Nickname_0 + WHERE [g].[Nickname] <> @__prm_Inner_Nickname_0 ) AS [g0] ORDER BY [g0].[FullName] """); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs index 7edb07d101a..04b95ffa244 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs @@ -3864,7 +3864,7 @@ public override async Task Query_reusing_parameter_doesnt_declare_duplicate_para FROM ( SELECT DISTINCT "g"."Nickname", "g"."SquadId", "g"."AssignedCityName", "g"."CityOfBirthName", "g"."Discriminator", "g"."FullName", "g"."HasSoulPatch", "g"."LeaderNickname", "g"."LeaderSquadId", "g"."Rank" FROM "Gears" AS "g" - WHERE "g"."Nickname" <> @__prm_Inner_Nickname_0 AND "g"."Nickname" <> @__prm_Inner_Nickname_0 + WHERE "g"."Nickname" <> @__prm_Inner_Nickname_0 ) AS "g0" ORDER BY "g0"."FullName" """);