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.

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

SqlServer: Block arithmetic on date/time expressions when type mapping is not known #28052

Merged
1 commit merged into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
/// </summary>
public class SqlServerSqlTranslatingExpressionVisitor : RelationalSqlTranslatingExpressionVisitor
{
private static readonly HashSet<string?> DateTimeDataTypes
private static readonly HashSet<string> DateTimeDataTypes
= new()
{
"time",
Expand All @@ -23,6 +23,16 @@ private static readonly HashSet<string?> DateTimeDataTypes
"datetimeoffset"
};

private static readonly HashSet<Type> DateTimeClrTypes
= new()
{
typeof(TimeOnly),
typeof(DateOnly),
typeof(TimeSpan),
typeof(DateTime),
typeof(DateTimeOffset)
};

private static readonly HashSet<ExpressionType> ArithmeticOperatorTypes
= new()
{
Expand Down Expand Up @@ -82,14 +92,32 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
}
}

return !(base.VisitBinary(binaryExpression) is SqlExpression visitedExpression)
? QueryCompilationContext.NotTranslatedExpression
: visitedExpression is SqlBinaryExpression sqlBinary
&& ArithmeticOperatorTypes.Contains(sqlBinary.OperatorType)
&& (DateTimeDataTypes.Contains(GetProviderType(sqlBinary.Left))
|| DateTimeDataTypes.Contains(GetProviderType(sqlBinary.Right)))
? QueryCompilationContext.NotTranslatedExpression
: visitedExpression;
var visitedExpression = base.VisitBinary(binaryExpression);

if (visitedExpression is SqlBinaryExpression sqlBinaryExpression
&& ArithmeticOperatorTypes.Contains(sqlBinaryExpression.OperatorType))
{
var inferredProviderType = GetProviderType(sqlBinaryExpression.Left) ?? GetProviderType(sqlBinaryExpression.Right);
if (inferredProviderType != null)
{
if (DateTimeDataTypes.Contains(inferredProviderType))
{
return QueryCompilationContext.NotTranslatedExpression;
}
}
else
{
var leftType = sqlBinaryExpression.Left.Type;
var rightType = sqlBinaryExpression.Right.Type;
if (DateTimeClrTypes.Contains(leftType)
|| DateTimeClrTypes.Contains(rightType))
{
return QueryCompilationContext.NotTranslatedExpression;
}
}
}

return visitedExpression;
}

/// <summary>
Expand Down Expand Up @@ -117,7 +145,7 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
isBinaryMaxDataType ? typeof(long) : typeof(int));

return isBinaryMaxDataType
? (Expression)Dependencies.SqlExpressionFactory.Convert(dataLengthSqlFunction, typeof(int))
? Dependencies.SqlExpressionFactory.Convert(dataLengthSqlFunction, typeof(int))
: dataLengthSqlFunction;
}

Expand Down
11 changes: 11 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5362,6 +5362,17 @@ public virtual Task DateTimeOffset_Contains_Less_than_Greater_than(bool async)
m => start <= m.Timeline.Date && m.Timeline < end && dates.Contains(m.Timeline)));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task DateTimeOffsetNow_minus_timespan(bool async)
{
var timeSpan = new TimeSpan(1000);

return AssertQuery(
async,
ss => ss.Set<Mission>().Where(e => e.Timeline > DateTimeOffset.Now - timeSpan));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Navigation_inside_interpolated_string_expanded(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5738,6 +5738,9 @@ FROM [Missions] AS [m]
WHERE @__start_0 <= CAST(CONVERT(date, [m].[Timeline]) AS datetimeoffset) AND [m].[Timeline] < @__end_1 AND [m].[Timeline] = '1902-01-02T10:00:00.1234567+01:30'");
}

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override async Task Navigation_inside_interpolated_string_expanded(bool async)
{
await base.Navigation_inside_interpolated_string_expanded(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8165,6 +8165,9 @@ FROM [Missions] AS [m]
WHERE @__start_0 <= CAST(CONVERT(date, [m].[Timeline]) AS datetimeoffset) AND [m].[Timeline] < @__end_1 AND [m].[Timeline] = '1902-01-02T10:00:00.1234567+01:30'");
}

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override async Task Navigation_inside_interpolated_string_expanded(bool async)
{
await base.Navigation_inside_interpolated_string_expanded(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6796,6 +6796,9 @@ FROM [Missions] AS [m]
WHERE @__start_0 <= CAST(CONVERT(date, [m].[Timeline]) AS datetimeoffset) AND [m].[Timeline] < @__end_1 AND [m].[Timeline] = '1902-01-02T10:00:00.1234567+01:30'");
}

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override async Task Navigation_inside_interpolated_string_expanded(bool async)
{
await base.Navigation_inside_interpolated_string_expanded(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5434,6 +5434,9 @@ public override async Task DateTimeOffset_Contains_Less_than_Greater_than(bool a
WHERE @__start_0 <= CAST(CONVERT(date, [m].[Timeline]) AS datetimeoffset) AND [m].[Timeline] < @__end_1 AND [m].[Timeline] = '1902-01-02T10:00:00.1234567+01:30'");
}

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override async Task Conditional_with_conditions_evaluating_to_false_gets_optimized(bool async)
{
await base.Conditional_with_conditions_evaluating_to_false_gets_optimized(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ public override async Task DateTimeOffset_Contains_Less_than_Greater_than(bool a
AssertSql();
}

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override async Task DateTimeOffset_Date_returns_datetime(bool async)
{
await AssertTranslationFailed(() => base.DateTimeOffset_Date_returns_datetime(async));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public override Task Where_datetimeoffset_year_component(bool async)
public override Task DateTimeOffset_Contains_Less_than_Greater_than(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffset_Contains_Less_than_Greater_than(async));

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override Task DateTimeOffset_Date_returns_datetime(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffset_Date_returns_datetime(async));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public override Task Where_datetimeoffset_year_component(bool async)
public override Task DateTimeOffset_Contains_Less_than_Greater_than(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffset_Contains_Less_than_Greater_than(async));

public override Task DateTimeOffsetNow_minus_timespan(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffsetNow_minus_timespan(async));

public override Task DateTimeOffset_Date_returns_datetime(bool async)
=> AssertTranslationFailed(() => base.DateTimeOffset_Date_returns_datetime(async));

Expand Down