Skip to content

Commit

Permalink
Query: Assign types for SqlExpression correctly
Browse files Browse the repository at this point in the history
Resolves #19990
When inferring type mapping, use inferred type mapping's clr type to determine resulting sqlExpression's type.
  • Loading branch information
smitpatel committed Apr 3, 2020
1 parent 8aef6b7 commit 402dce1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 11 deletions.
24 changes: 16 additions & 8 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ private SqlExpression ApplyTypeMappingOnSqlUnary(
SqlUnaryExpression sqlUnaryExpression, RelationalTypeMapping typeMapping)
{
SqlExpression operand;
Type resultType;
RelationalTypeMapping resultTypeMapping;
switch (sqlUnaryExpression.OperatorType)
{
Expand All @@ -115,30 +116,31 @@ private SqlExpression ApplyTypeMappingOnSqlUnary(
when sqlUnaryExpression.IsLogicalNot():
{
resultTypeMapping = _boolTypeMapping;
resultType = typeof(bool);
operand = ApplyDefaultTypeMapping(sqlUnaryExpression.Operand);
break;
}

case ExpressionType.Convert:
resultTypeMapping = typeMapping;
// Since we are applying convert, resultTypeMapping decides the clrType
resultType = resultTypeMapping?.ClrType ?? sqlUnaryExpression.Type;
operand = ApplyDefaultTypeMapping(sqlUnaryExpression.Operand);
break;

case ExpressionType.Not:
case ExpressionType.Negate:
resultTypeMapping = typeMapping;
// While Not is logical, negate is numeric hence we use clrType from TypeMapping
resultType = resultTypeMapping?.ClrType ?? sqlUnaryExpression.Type;
operand = ApplyTypeMapping(sqlUnaryExpression.Operand, typeMapping);
break;

default:
throw new InvalidOperationException(CoreStrings.UnsupportedUnary);
}

return new SqlUnaryExpression(
sqlUnaryExpression.OperatorType,
operand,
sqlUnaryExpression.Type,
resultTypeMapping);
return new SqlUnaryExpression(sqlUnaryExpression.OperatorType, operand, resultType, resultTypeMapping);
}

private SqlExpression ApplyTypeMappingOnSqlBinary(
Expand All @@ -160,7 +162,10 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
case ExpressionType.NotEqual:
{
inferredTypeMapping = ExpressionExtensions.InferTypeMapping(left, right)
?? _typeMappingSource.FindMapping(left.Type);
// We avoid object here since the result does not get typeMapping from outside.
?? (left.Type != typeof(object)
? _typeMappingSource.FindMapping(left.Type)
: _typeMappingSource.FindMapping(right.Type));
resultType = typeof(bool);
resultTypeMapping = _boolTypeMapping;
break;
Expand All @@ -184,7 +189,7 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
case ExpressionType.Or:
{
inferredTypeMapping = typeMapping ?? ExpressionExtensions.InferTypeMapping(left, right);
resultType = left.Type;
resultType = inferredTypeMapping?.ClrType ?? left.Type;
resultTypeMapping = inferredTypeMapping;
break;
}
Expand Down Expand Up @@ -437,7 +442,10 @@ public virtual CaseExpression Case(

var operandTypeMapping = operand.TypeMapping
?? whenClauses.Select(wc => wc.Test.TypeMapping).FirstOrDefault(t => t != null)
?? _typeMappingSource.FindMapping(operand.Type);
// Since we never look at type of Operand/Test after this place,
// we need to find actual typeMapping based on non-object type.
?? new[] { operand.Type }.Concat(whenClauses.Select(wc => wc.Test.Type))
.Where(t => t != typeof(object)).Select(t => _typeMappingSource.FindMapping(t)).FirstOrDefault();

var resultTypeMapping = elseResult?.TypeMapping
?? whenClauses.Select(wc => wc.Result.TypeMapping).FirstOrDefault(t => t != null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2778,7 +2778,7 @@ public override async Task Concat_string_int(bool async)
await base.Concat_string_int(async);

AssertSql(
@"SELECT CAST([o].[OrderID] AS nchar(5)) + [o].[CustomerID]
@"SELECT CAST([o].[OrderID] AS nchar(5)) + COALESCE([o].[CustomerID], N'')
FROM [Orders] AS [o]");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1341,7 +1341,7 @@ public override async Task Where_concat_string_int_comparison4(bool async)
AssertSql(
@"SELECT [o].[CustomerID]
FROM [Orders] AS [o]
WHERE ((CAST([o].[OrderID] AS nchar(5)) + [o].[CustomerID]) = [o].[CustomerID]) OR [o].[CustomerID] IS NULL");
WHERE (CAST([o].[OrderID] AS nchar(5)) + COALESCE([o].[CustomerID], N'')) = [o].[CustomerID]");
}

public override async Task Where_concat_string_string_comparison(bool async)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,13 @@ public override Task Complex_nested_query_doesnt_try_binding_to_grandparent_when

public override Task AsQueryable_in_query_server_evals(bool async) => null;

[ConditionalTheory(Skip = "Issue #19990")]
public override async Task Concat_string_int(bool async)
{
await base.Concat_string_int(async);

AssertSql(
@"SELECT CAST(""o"".""OrderID"" AS TEXT) || COALESCE(""o"".""CustomerID"", '')
FROM ""Orders"" AS ""o""");
}

public override async Task Concat_int_string(bool async)
Expand Down

0 comments on commit 402dce1

Please sign in to comment.