Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TINY] Fix to #16245 - Null semantics applies incorrectly for SqlFunction #16266

Merged
merged 2 commits into from
Jun 28, 2019
Merged

[TINY] Fix to #16245 - Null semantics applies incorrectly for SqlFunction #16266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8502ee
Fix to #16245 - Null semantics applies incorrectly for SqlFunction
maumar Jun 26, 2019
d3b351f
Expand fix to reset nullability for all the Visit* methods
smitpatel Jun 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/EFCore.Relational/Query/Pipeline/NullSemanticsRewritingVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ private SqlParameterExpression VisitSqlParameterExpression(SqlParameterExpressio

private SqlUnaryExpression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
{
_isNullable = false;
var newOperand = (SqlExpression)Visit(sqlUnaryExpression.Operand);

// IsNull/IsNotNull
// IsNull/IsNotNull
if (sqlUnaryExpression.OperatorType == ExpressionType.Equal
|| sqlUnaryExpression.OperatorType == ExpressionType.NotEqual)
{
Expand All @@ -103,6 +104,7 @@ private SqlUnaryExpression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryEx

private LikeExpression VisitLikeExpression(LikeExpression likeExpression)
{
_isNullable = false;
var newMatch = (SqlExpression)Visit(likeExpression.Match);
var isNullable = _isNullable;
var newPattern = (SqlExpression)Visit(likeExpression.Pattern);
Expand Down Expand Up @@ -149,6 +151,7 @@ private SqlExpression VisitJoinPredicate(SqlBinaryExpression predicate)

private CaseExpression VisitCaseExpression(CaseExpression caseExpression)
{
_isNullable = false;
// if there is no 'else' there is a possibility of null, when none of the conditions are met
// otherwise the result is nullable if any of the WhenClause results OR ElseResult is nullable
var isNullable = caseExpression.ElseResult == null;
Expand All @@ -171,6 +174,7 @@ private CaseExpression VisitCaseExpression(CaseExpression caseExpression)

private SqlFunctionExpression VisitSqlFunctionExpression(SqlFunctionExpression sqlFunctionExpression)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CaseExpression may also need reset since Operand can be null.

{
_isNullable = false;
var newInstance = (SqlExpression)Visit(sqlFunctionExpression.Instance);
var isNullable = _isNullable;
var newArguments = new SqlExpression[sqlFunctionExpression.Arguments.Count];
Expand All @@ -187,9 +191,11 @@ private SqlFunctionExpression VisitSqlFunctionExpression(SqlFunctionExpression s

private SqlBinaryExpression VisitSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpression)
{
_isNullable = false;
var newLeft = (SqlExpression)Visit(sqlBinaryExpression.Left);
var leftNullable = _isNullable;

_isNullable = false;
var newRight = (SqlExpression)Visit(sqlBinaryExpression.Right);
var rightNullable = _isNullable;

Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Where.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,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 ((CONVERT(date, GETDATE()) = CONVERT(date, GETDATE())) AND (CONVERT(date, GETDATE()) IS NOT NULL AND CONVERT(date, GETDATE()) IS NOT NULL)) OR (CONVERT(date, GETDATE()) IS NULL AND CONVERT(date, GETDATE()) IS NULL)");
WHERE CONVERT(date, GETDATE()) = CONVERT(date, GETDATE())");
}

public override async Task Where_datetime_date_component(bool isAsync)
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions test/EFCore.Sqlite.FunctionalTests/Query/SimpleQuerySqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down