Skip to content

Commit

Permalink
Fix to #19128 - Query: Error for queries with enum parameters whose v…
Browse files Browse the repository at this point in the history
…alue is of underlying type but the value expected by type mapping is the actual enum type

Problem was that we were not correctly compensating for type differences between parameter value and the DbParameter type (determined from type mapping). This can happen when the parameter is explicitly typed as the underlying type, but in the query the type is inferred from other side of binary expression etc.

Fix is to detect the case when expected parameter value is Enum, but the actual type is the underlying type and convert the value back to the enum type, just like we do for constants.
  • Loading branch information
maumar committed Mar 24, 2020
1 parent 6f4f078 commit a0dae0d
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 9 deletions.
28 changes: 19 additions & 9 deletions src/EFCore.Relational/Storage/RelationalTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace Microsoft.EntityFrameworkCore.Storage
/// </summary>
public abstract class RelationalTypeMapping : CoreTypeMapping
{
private readonly bool _quirk19128;

/// <summary>
/// Parameter object for use in the <see cref="RelationalTypeMapping" /> hierarchy.
/// </summary>
Expand Down Expand Up @@ -254,6 +256,8 @@ protected RelationalTypeMapping(RelationalTypeMappingParameters parameters)
}

StoreType = storeType;

_quirk19128 = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue19825", out var enabled) && enabled;
}

/// <summary>
Expand Down Expand Up @@ -435,6 +439,11 @@ public virtual DbParameter CreateParameter(
parameter.Direction = ParameterDirection.Input;
parameter.ParameterName = name;

if (!_quirk19128)
{
value = ConvertUnderlyingEnumValueToEnum(value);
}

if (Converter != null)
{
value = Converter.ConvertToProvider(value);
Expand All @@ -457,6 +466,15 @@ public virtual DbParameter CreateParameter(
return parameter;
}

// Enum when compared to constant will always have value of integral type
// when enum would contain convert node. We remove the convert node but we also
// need to convert the integral value to enum value.
// This allows us to use converter on enum value or print enum value directly if supported by provider
private object ConvertUnderlyingEnumValueToEnum(object value)
=> value?.GetType().IsInteger() == true && ClrType.UnwrapNullableType().IsEnum
? Enum.ToObject(ClrType.UnwrapNullableType(), value)
: value;

/// <summary>
/// Configures type information of a <see cref="DbParameter" />.
/// </summary>
Expand All @@ -474,15 +492,7 @@ protected virtual void ConfigureParameter([NotNull] DbParameter parameter)
/// </returns>
public virtual string GenerateSqlLiteral([CanBeNull] object value)
{
// Enum when compared to constant will always have constant of integral type
// when enum would contain convert node. We remove the convert node but we also
// need to convert the integral value to enum value.
// This allows us to use converter on enum value or print enum value directly if supported by provider
if (value?.GetType().IsInteger() == true
&& ClrType.UnwrapNullableType().IsEnum)
{
value = Enum.ToObject(ClrType.UnwrapNullableType(), value);
}
value = ConvertUnderlyingEnumValueToEnum(value);

if (Converter != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,11 @@ public override Task Select_subquery_projecting_single_constant_inside_anonymous
{
return base.Select_subquery_projecting_single_constant_inside_anonymous(isAsync);
}

[ConditionalTheory(Skip = "issue #18284")]
public override Task Enum_closure_typed_as_underlying_type_generates_correct_parameter_type(bool async)
{
return base.Enum_closure_typed_as_underlying_type_generates_correct_parameter_type(async);
}
}
}
48 changes: 48 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7595,6 +7595,54 @@ public virtual Task DateTimeOffset_Date_returns_datetime(bool async)
ss => ss.Set<Mission>().Where(m => m.Timeline.Date >= dateTimeOffset.Date));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Enum_closure_typed_as_underlying_type_generates_correct_parameter_type(bool async)
{
var prm = (int)AmmunitionType.Cartridge;

return AssertQuery(
async,
ss => ss.Set<Weapon>().Where(w => prm == (int)w.AmmunitionType),
ss => ss.Set<Weapon>().Where(w => w.AmmunitionType != null && prm == (int)w.AmmunitionType));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Enum_flags_closure_typed_as_underlying_type_generates_correct_parameter_type(bool async)
{
var prm = (int)MilitaryRank.Private + (int)MilitaryRank.Sergeant + (int)MilitaryRank.General;

return AssertQuery(
async,
ss => ss.Set<Gear>()
.Where(g => (prm & (int)g.Rank) == (int)g.Rank));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Enum_flags_closure_typed_as_different_type_generates_correct_parameter_type(bool async)
{
var prm = (byte)MilitaryRank.Private + (byte)MilitaryRank.Sergeant;

return AssertQuery(
async,
ss => ss.Set<Gear>()
.Where(g => (prm & (short)g.Rank) == (short)g.Rank));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Constant_enum_with_same_underlying_value_as_previously_parameterized_int(bool async)
{
return AssertQueryScalar(
async,
ss => ss.Set<Gear>()
.OrderBy(g => g.Nickname)
.Take(1)
.Select(g => g.Rank & MilitaryRank.Corporal));
}

protected GearsOfWarContext CreateContext() => Fixture.CreateContext();

protected virtual void ClearLog()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7562,6 +7562,55 @@ FROM [Missions] AS [m]
WHERE CONVERT(date, [m].[Timeline]) >= @__dateTimeOffset_Date_0");
}

public override async Task Enum_closure_typed_as_underlying_type_generates_correct_parameter_type(bool async)
{
await base.Enum_closure_typed_as_underlying_type_generates_correct_parameter_type(async);

AssertSql(
@"@__prm_0='1'
SELECT [w].[Id], [w].[AmmunitionType], [w].[IsAutomatic], [w].[Name], [w].[OwnerFullName], [w].[SynergyWithId]
FROM [Weapons] AS [w]
WHERE @__prm_0 = [w].[AmmunitionType]");
}

public override async Task Enum_flags_closure_typed_as_underlying_type_generates_correct_parameter_type(bool async)
{
await base.Enum_flags_closure_typed_as_underlying_type_generates_correct_parameter_type(async);

AssertSql(
@"@__prm_0='66'
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
FROM [Gears] AS [g]
WHERE [g].[Discriminator] IN (N'Gear', N'Officer') AND ((@__prm_0 & [g].[Rank]) = [g].[Rank])");
}

public override async Task Enum_flags_closure_typed_as_different_type_generates_correct_parameter_type(bool async)
{
await base.Enum_flags_closure_typed_as_different_type_generates_correct_parameter_type(async);

AssertSql(
@"@__prm_0='2'
SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank]
FROM [Gears] AS [g]
WHERE [g].[Discriminator] IN (N'Gear', N'Officer') AND ((@__prm_0 & CAST([g].[Rank] AS int)) = CAST([g].[Rank] AS int))");
}

public override async Task Constant_enum_with_same_underlying_value_as_previously_parameterized_int(bool async)
{
await base.Constant_enum_with_same_underlying_value_as_previously_parameterized_int(async);

AssertSql(
@"@__p_0='1'
SELECT TOP(@__p_0) [g].[Rank] & @__p_0
FROM [Gears] AS [g]
WHERE [g].[Discriminator] IN (N'Gear', N'Officer')
ORDER BY [g].[Nickname]");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
Expand Down

0 comments on commit a0dae0d

Please sign in to comment.