From 1163dba5d5ffa29ba758c0d5693d5d50136e47d7 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Wed, 26 Feb 2020 14:05:37 -0800 Subject: [PATCH] Fix to #19128 - Query: Error for queries with enum parameters whose value 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 --- .../Storage/RelationalTypeMapping.cs | 21 ++++---- .../Query/GearsOfWarQueryInMemoryTest.cs | 6 +++ .../Query/GearsOfWarQueryTestBase.cs | 48 ++++++++++++++++++ .../Query/GearsOfWarQuerySqlServerTest.cs | 49 +++++++++++++++++++ 4 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs index cd4d7813860..763759ac5c6 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs @@ -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); @@ -456,6 +458,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; + /// /// Configures type information of a . /// @@ -473,15 +484,7 @@ protected virtual void ConfigureParameter([NotNull] DbParameter parameter) /// 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) { diff --git a/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs index 3494e10dfd2..e763cb39903 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs @@ -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); + } } } diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index c436c8db3e7..5501db8f5df 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -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().Where(w => prm == (int)w.AmmunitionType), + ss => ss.Set().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() + .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() + .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() + .OrderBy(g => g.Nickname) + .Take(1) + .Select(g => g.Rank & MilitaryRank.Private)); + } + protected GearsOfWarContext CreateContext() => Fixture.CreateContext(); protected virtual void ClearLog() diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs index 3bab0d87056..e850e333e80 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs @@ -7568,6 +7568,55 @@ 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( + @"@__prm_0='5' + +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); }