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.
dotnet · Jun 26, 2019 · 0ca1d03 · 0ca1d03
1 parent 511d365
commit 0ca1d03
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 9 deletions.
diff --git a/src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs b/src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs
@@ -171,6 +171,7 @@ private CaseExpression VisitCaseExpression(CaseExpression caseExpression)
 
         private SqlFunctionExpression VisitSqlFunctionExpression(SqlFunctionExpression sqlFunctionExpression)
         {
+            _isNullable = false;
             var newInstance = (SqlExpression)Visit(sqlFunctionExpression.Instance);
             var isNullable = _isNullable;
             var newArguments = new SqlExpression[sqlFunctionExpression.Arguments.Count];

diff --git 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
@@ -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
@@ -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)