From 8eb8fb0fed9a5e659dee283b2b7ff32e1efbee6a Mon Sep 17 00:00:00 2001 From: maumar Date: Tue, 25 Jun 2019 17:17:22 -0700 Subject: [PATCH] Fix to #16245 - Null semantics applies incorrectly for SqlFunction problem was that when we were computing a nullability of sql function, if the function didn't have instance, we would inherit nullability from the expression that was visited previously. Fix is to only use nullability of instance expression if that expression is not null. --- .../Query/Pipeline/NullSemanticsRewritingVisitor.cs | 10 ++++++++-- .../Query/SimpleQuerySqlServerTest.Functions.cs | 6 +++--- .../Query/SimpleQuerySqlServerTest.cs | 2 +- .../Query/SimpleQuerySqliteTest.cs | 10 +++++----- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs b/src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs index 6a65d27986b..66b6d8c61c3 100644 --- a/src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs +++ b/src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs @@ -171,8 +171,14 @@ private CaseExpression VisitCaseExpression(CaseExpression caseExpression) private SqlFunctionExpression VisitSqlFunctionExpression(SqlFunctionExpression sqlFunctionExpression) { - var newInstance = (SqlExpression)Visit(sqlFunctionExpression.Instance); - var isNullable = _isNullable; + var newInstance = default(SqlExpression); + var isNullable = false; + if (sqlFunctionExpression.Instance != null) + { + newInstance = (SqlExpression)Visit(sqlFunctionExpression.Instance); + isNullable = _isNullable; + } + var newArguments = new SqlExpression[sqlFunctionExpression.Arguments.Count]; for (var i = 0; i < newArguments.Length; i++) { diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs index f2a8452a15c..cd516be73a3 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs @@ -843,7 +843,7 @@ public override async Task Where_string_to_upper(bool isAsync) AssertSql( @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c] -WHERE (UPPER([c].[CustomerID]) = N'ALFKI') AND UPPER([c].[CustomerID]) IS NOT NULL"); +WHERE UPPER([c].[CustomerID]) = N'ALFKI'"); } public override async Task Where_string_to_lower(bool isAsync) @@ -853,7 +853,7 @@ public override async Task Where_string_to_lower(bool isAsync) AssertSql( @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c] -WHERE (LOWER([c].[CustomerID]) = N'alfki') AND LOWER([c].[CustomerID]) IS NOT NULL"); +WHERE LOWER([c].[CustomerID]) = N'alfki'"); } public override async Task Where_functions_nested(bool isAsync) @@ -863,7 +863,7 @@ public override async Task Where_functions_nested(bool isAsync) AssertSql( @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c] -WHERE (POWER(CAST(CAST(LEN([c].[CustomerID]) AS int) AS float), 2.0E0) = 25.0E0) AND POWER(CAST(CAST(LEN([c].[CustomerID]) AS int) AS float), 2.0E0) IS NOT NULL"); +WHERE POWER(CAST(CAST(LEN([c].[CustomerID]) AS int) AS float), 2.0E0) = 25.0E0"); } public override async Task Convert_ToByte(bool isAsync) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.cs index e92efd08857..cad3024874e 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.cs @@ -1153,7 +1153,7 @@ public override async Task Null_conditional_deep(bool isAsync) AssertSql( @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c] -WHERE (CAST(LEN([c].[CustomerID]) AS int) = 5) AND CAST(LEN([c].[CustomerID]) AS int) IS NOT NULL"); +WHERE CAST(LEN([c].[CustomerID]) AS int) = 5"); } public override async Task Queryable_simple(bool isAsync) diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs index 7ec5989c51a..8647165eeb5 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs @@ -204,7 +204,7 @@ public override async Task Where_datetime_now(bool isAsync) SELECT ""c"".""CustomerID"", ""c"".""Address"", ""c"".""City"", ""c"".""CompanyName"", ""c"".""ContactName"", ""c"".""ContactTitle"", ""c"".""Country"", ""c"".""Fax"", ""c"".""Phone"", ""c"".""PostalCode"", ""c"".""Region"" FROM ""Customers"" AS ""c"" -WHERE ((rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime'), '0'), '.') <> @__myDatetime_0) OR (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime'), '0'), '.') IS NULL OR @__myDatetime_0 IS NULL)) AND (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime'), '0'), '.') IS NOT NULL OR @__myDatetime_0 IS NOT NULL)"); +WHERE (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime'), '0'), '.') <> @__myDatetime_0) OR @__myDatetime_0 IS NULL"); } public override async Task Where_datetime_utcnow(bool isAsync) @@ -216,7 +216,7 @@ public override async Task Where_datetime_utcnow(bool isAsync) SELECT ""c"".""CustomerID"", ""c"".""Address"", ""c"".""City"", ""c"".""CompanyName"", ""c"".""ContactName"", ""c"".""ContactTitle"", ""c"".""Country"", ""c"".""Fax"", ""c"".""Phone"", ""c"".""PostalCode"", ""c"".""Region"" FROM ""Customers"" AS ""c"" -WHERE ((rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now'), '0'), '.') <> @__myDatetime_0) OR (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now'), '0'), '.') IS NULL OR @__myDatetime_0 IS NULL)) AND (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now'), '0'), '.') IS NOT NULL OR @__myDatetime_0 IS NOT NULL)"); +WHERE (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now'), '0'), '.') <> @__myDatetime_0) OR @__myDatetime_0 IS NULL"); } public override async Task Where_datetime_today(bool isAsync) @@ -226,7 +226,7 @@ public override async Task Where_datetime_today(bool isAsync) AssertSql( @"SELECT ""e"".""EmployeeID"", ""e"".""City"", ""e"".""Country"", ""e"".""FirstName"", ""e"".""ReportsTo"", ""e"".""Title"" FROM ""Employees"" AS ""e"" -WHERE ((rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.') = rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.')) AND (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.') IS NOT NULL AND rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.') IS NOT NULL)) OR (rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.') IS NULL AND rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.') IS NULL)"); +WHERE rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.') = rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', 'now', 'localtime', 'start of day'), '0'), '.')"); } public override async Task Where_datetime_date_component(bool isAsync) @@ -631,7 +631,7 @@ public override async Task Where_string_to_lower(bool isAsync) AssertSql( @"SELECT ""c"".""CustomerID"", ""c"".""Address"", ""c"".""City"", ""c"".""CompanyName"", ""c"".""ContactName"", ""c"".""ContactTitle"", ""c"".""Country"", ""c"".""Fax"", ""c"".""Phone"", ""c"".""PostalCode"", ""c"".""Region"" FROM ""Customers"" AS ""c"" -WHERE (lower(""c"".""CustomerID"") = 'alfki') AND lower(""c"".""CustomerID"") IS NOT NULL"); +WHERE lower(""c"".""CustomerID"") = 'alfki'"); } public override async Task Where_string_to_upper(bool isAsync) @@ -641,7 +641,7 @@ public override async Task Where_string_to_upper(bool isAsync) AssertSql( @"SELECT ""c"".""CustomerID"", ""c"".""Address"", ""c"".""City"", ""c"".""CompanyName"", ""c"".""ContactName"", ""c"".""ContactTitle"", ""c"".""Country"", ""c"".""Fax"", ""c"".""Phone"", ""c"".""PostalCode"", ""c"".""Region"" FROM ""Customers"" AS ""c"" -WHERE (upper(""c"".""CustomerID"") = 'ALFKI') AND upper(""c"".""CustomerID"") IS NOT NULL"); +WHERE upper(""c"".""CustomerID"") = 'ALFKI'"); } public override async Task TrimStart_without_arguments_in_predicate(bool isAsync)