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.

Fixes #19128
  • Loading branch information
maumar committed Feb 26, 2020
1 parent e73bdd9 commit ab13a29
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 11 deletions.
21 changes: 12 additions & 9 deletions src/EFCore.Relational/Storage/RelationalTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ public virtual DbParameter CreateParameter(
parameter.Direction = ParameterDirection.Input;
parameter.ParameterName = name;

value = ConvertUnderlyingEnumValueToEnum(value);

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

// 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
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 @@ -473,15 +484,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
4 changes: 2 additions & 2 deletions src/EFCore/Storage/ValueConversion/ValueConverter`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ private static Func<object, object> SanitizeConverter<TIn, TOut>(Expression<Func
var compiled = convertExpression.Compile();

return v => v == null
? (object)null
: compiled(Sanitize<TIn>(v));
? (object)null
: compiled(Sanitize<TIn>(v));
}

private static T Sanitize<T>(object value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,11 @@ public override Task Group_by_on_StartsWith_with_null_parameter_as_argument(bool
{
return base.Group_by_on_StartsWith_with_null_parameter_as_argument(async);
}

[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 @@ -7362,6 +7362,54 @@ public virtual Task Where_TimeSpan_Milliseconds(bool async)
.Where(m => m.Duration.Milliseconds == 1));
}

[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.Private));
}

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

protected virtual void ClearLog()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7568,6 +7568,51 @@ FROM [Missions] AS [m]
WHERE DATEPART(millisecond, [m].[Duration]) = 1");
}

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='133'
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(
@"");
}

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
56 changes: 56 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7109,6 +7109,62 @@ private class CustomerView19708

#endregion

//[ConditionalFact]

//public void Test19128()
//{
// var db = new LeadsDbContext();
// db.Database.EnsureDeleted();
// db.Database.EnsureCreated();
// var downloadLead = new Lead { LeadType = LeadType.Download };
// var productLead = new Lead { LeadType = LeadType.Product };
// var leads = new List<Lead> { downloadLead, productLead };
// db.AddRange(leads);
// db.SaveChanges();

// //fails
// var sumLeadTypes = (int)LeadType.Download + (int)LeadType.Product;
// var matchingLeads = db.Leads
// .Where(l => (sumLeadTypes & (int)l.LeadType) == (int)l.LeadType)
// .ToList();

// // works
// //var sumLeadTypes2 = LeadType.Download | LeadType.Product;
// //var matchingLeads2 = db.Leads
// // .Where(l => (sumLeadTypes2 & l.LeadType) == l.LeadType)
// // .ToList();
//}

//public class Lead
//{
// [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
// public int Id { get; set; }
// public LeadType LeadType { get; set; }
//}

//[Flags]
//public enum LeadType : short
//{
// PremiumContent = 1,
// ActiveProjectLeads = 2,
// Product = 4,
// ContactRequest = 8,
// Shared = 16,
// Download = 32,
// //InternalHelpRequest = 64,
// //Article = 128
//}

//public class LeadsDbContext : DbContext
//{
// public DbSet<Lead> Leads { get; set; }

// protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
// {
// optionsBuilder.UseSqlServer(@"Server=.;Database=Repro19128;Trusted_Connection=True;MultipleActiveResultSets=True");
// }
//}

private DbContextOptions _options;

private SqlServerTestStore CreateTestStore<TContext>(
Expand Down

0 comments on commit ab13a29

Please sign in to comment.