From 4795eee89fd67a05ae3aa288509c57200f0d7c89 Mon Sep 17 00:00:00 2001 From: Jon Louie Date: Tue, 30 Jun 2020 03:56:48 -0700 Subject: [PATCH] Fix Issue 13919: Translation error using negative (#21389) * create negation unit tests * add parentheses to negated expression on translation * expose unit test to sqlite test suite * add unit tests for negating a column and LIKE expression * use RequiresBrackets helper Fixes #13919 --- .../Query/QuerySqlGenerator.cs | 10 +++++++ .../Query/GearsOfWarQueryTestBase.cs | 27 +++++++++++++++++ .../Query/GearsOfWarQuerySqlServerTest.cs | 30 +++++++++++++++++++ .../Query/GearsOfWarQuerySqliteTest.cs | 30 +++++++++++++++++++ 4 files changed, 97 insertions(+) diff --git a/src/EFCore.Relational/Query/QuerySqlGenerator.cs b/src/EFCore.Relational/Query/QuerySqlGenerator.cs index d47258e841b..66ee4f2aefd 100644 --- a/src/EFCore.Relational/Query/QuerySqlGenerator.cs +++ b/src/EFCore.Relational/Query/QuerySqlGenerator.cs @@ -738,7 +738,17 @@ when sqlUnaryExpression.Type.UnwrapNullableType() == typeof(bool): case ExpressionType.Negate: { _relationalCommandBuilder.Append("-"); + var requiresBrackets = RequiresBrackets(sqlUnaryExpression.Operand); + if (requiresBrackets) + { + _relationalCommandBuilder.Append("("); + } + Visit(sqlUnaryExpression.Operand); + if (requiresBrackets) + { + _relationalCommandBuilder.Append(")"); + } break; } } diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index 3fda9dc8e1b..eff2b8ac670 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -36,6 +36,33 @@ protected GearsOfWarQueryTestBase(TFixture fixture) { } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Negate_on_binary_expression(bool async) + { + return AssertQuery( + async, + ss => ss.Set().Where(s => s.Id == -(s.Id + s.Id))); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Negate_on_column(bool async) + { + return AssertQuery( + async, + ss => ss.Set().Where(s => s.Id == -s.Id)); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Negate_on_like_expression(bool async) + { + return AssertQuery( + async, + ss => ss.Set().Where(s => !s.Name.StartsWith("us"))); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Entity_equality_empty(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs index adcc51f0a5b..aa765286fa9 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs @@ -23,6 +23,36 @@ public GearsOfWarQuerySqlServerTest(GearsOfWarQuerySqlServerFixture fixture, ITe protected override bool CanExecuteQueryString => true; + public override async Task Negate_on_binary_expression(bool async) + { + await base.Negate_on_binary_expression(async); + + AssertSql( + @"SELECT [s].[Id], [s].[Banner], [s].[Banner5], [s].[InternalNumber], [s].[Name] +FROM [Squads] AS [s] +WHERE [s].[Id] = -([s].[Id] + [s].[Id])"); + } + + public override async Task Negate_on_column(bool async) + { + await base.Negate_on_column(async); + + AssertSql( + @"SELECT [s].[Id], [s].[Banner], [s].[Banner5], [s].[InternalNumber], [s].[Name] +FROM [Squads] AS [s] +WHERE [s].[Id] = -[s].[Id]"); + } + + public override async Task Negate_on_like_expression(bool async) + { + await base.Negate_on_like_expression(async); + + AssertSql( + @"SELECT [s].[Id], [s].[Banner], [s].[Banner5], [s].[InternalNumber], [s].[Name] +FROM [Squads] AS [s] +WHERE [s].[Name] IS NOT NULL AND NOT ([s].[Name] LIKE N'us%')"); + } + public override async Task Entity_equality_empty(bool async) { await base.Entity_equality_empty(async); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs index 5d4c6e82426..a8873265de9 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs @@ -78,6 +78,36 @@ public override Task DateTimeOffset_Date_returns_datetime(bool async) public override Task Subquery_projecting_non_nullable_scalar_contains_non_nullable_value_doesnt_need_null_expansion_negated(bool async) => null; + public override async Task Negate_on_binary_expression(bool async) + { + await base.Negate_on_binary_expression(async); + + AssertSql( + @"SELECT ""s"".""Id"", ""s"".""Banner"", ""s"".""Banner5"", ""s"".""InternalNumber"", ""s"".""Name"" +FROM ""Squads"" AS ""s"" +WHERE ""s"".""Id"" = -(""s"".""Id"" + ""s"".""Id"")"); + } + + public override async Task Negate_on_column(bool async) + { + await base.Negate_on_column(async); + + AssertSql( + @"SELECT ""s"".""Id"", ""s"".""Banner"", ""s"".""Banner5"", ""s"".""InternalNumber"", ""s"".""Name"" +FROM ""Squads"" AS ""s"" +WHERE ""s"".""Id"" = -""s"".""Id"""); + } + + public override async Task Negate_on_like_expression(bool async) + { + await base.Negate_on_like_expression(async); + + AssertSql( + @"SELECT ""s"".""Id"", ""s"".""Banner"", ""s"".""Banner5"", ""s"".""InternalNumber"", ""s"".""Name"" +FROM ""Squads"" AS ""s"" +WHERE ""s"".""Name"" IS NOT NULL AND NOT (""s"".""Name"" LIKE 'us%')"); + } + public override async Task Select_datetimeoffset_comparison_in_projection(bool async) { await base.Select_datetimeoffset_comparison_in_projection(async);