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.

Sign up for GitHub

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

More fixes to entity equality nullability handling #17058

Merged
merged 1 commit into from
Aug 9, 2019
Merged

More fixes to entity equality nullability handling #17058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions src/EFCore/Query/Internal/EntityEqualityRewritingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ protected virtual Expression VisitContainsMethodCall(MethodCallExpression method
{
// The source list is a constant, evaluate and replace with a list of the keys
var listValue = (IEnumerable)listConstant.Value;
var keyListType = typeof(List<>).MakeGenericType(keyProperty.ClrType);
var keyListType = typeof(List<>).MakeGenericType(keyProperty.ClrType.MakeNullable());
var keyList = (IList)Activator.CreateInstance(keyListType);
var getter = keyProperty.GetGetter();
foreach (var listItem in listValue)
Expand All @@ -386,7 +386,6 @@ protected virtual Expression VisitContainsMethodCall(MethodCallExpression method
&& listParam.Name.StartsWith(CompiledQueryCache.CompiledQueryParameterPrefix, StringComparison.Ordinal))
{
// The source list is a parameter. Add a runtime parameter that will contain a list of the extracted keys for each execution.
var keyListType = typeof(List<>).MakeGenericType(keyProperty.ClrType);
var lambda = Expression.Lambda(
Expression.Call(
_parameterListValueExtractor.MakeGenericMethod(entityType.ClrType, keyProperty.ClrType.MakeNullable()),
Expand All @@ -397,7 +396,10 @@ protected virtual Expression VisitContainsMethodCall(MethodCallExpression method
);

var newParameterName = $"{RuntimeParameterPrefix}{listParam.Name.Substring(CompiledQueryCache.CompiledQueryParameterPrefix.Length)}_{keyProperty.Name}";
rewrittenSource = _queryCompilationContext.RegisterRuntimeParameter(newParameterName, lambda, keyListType);
rewrittenSource = _queryCompilationContext.RegisterRuntimeParameter(
newParameterName,
lambda,
typeof(List<>).MakeGenericType(keyProperty.ClrType.MakeNullable()));
}
else
{
Expand Down Expand Up @@ -911,8 +913,6 @@ private static readonly MethodInfo _parameterValueExtractor
/// </summary>
private static List<TProperty> ParameterListValueExtractor<TEntity, TProperty>(QueryContext context, string baseParameterName, IProperty property)
{
Debug.Assert(property.ClrType == typeof(TProperty));

var baseListParameter = context.ParameterValues[baseParameterName] as IEnumerable<TEntity>;
if (baseListParameter == null)
{
Expand Down
22 changes: 22 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.ResultOperators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1278,6 +1278,28 @@ FROM root c
WHERE ((c[""Discriminator""] = ""Order"") AND (c[""OrderID""] = 10248))");
}

[ConditionalTheory(Skip = "Issue#14935 (Contains not implemented)")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Contains does not have translation in cosmos. It is less of not implemented and more of client eval.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was just following the way previous Contains tests were skipped. Am going to merge as-is, and can submit a PR to return Task.Complete instead of skipping.

/cc @AndriySvyryd

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's alright. We want skip only rather than Task.Completed. It's just skip message was somewhat misleading. But test code so no big deal.

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

AssertSql(
@"SELECT c
FROM root c
WHERE ((c[""Discriminator""] = ""Order"") AND (c[""OrderID""] = 10248))");
}

[ConditionalTheory(Skip = "Issue#14935 (Contains not implemented)")]
public override async Task Contains_with_constant_list_value_type_id(bool isAsync)
{
await base.Contains_with_constant_list_value_type_id(isAsync);

AssertSql(
@"SELECT c
FROM root c
WHERE ((c[""Discriminator""] = ""Order"") AND (c[""OrderID""] = 10248))");
}

[ConditionalTheory(Skip = "Issue#14935 (Contains not implemented)")]
public override void Contains_over_entityType_with_null_should_rewrite_to_identity_equality()
{
Expand Down
26 changes: 26 additions & 0 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.ResultOperators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,32 @@ public virtual Task List_Contains_with_parameter_list(bool isAsync)
entryCount: 2);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_with_parameter_list_value_type_id(bool isAsync)
{
var orders = new List<Order>
{
new Order { OrderID = 10248 },
new Order { OrderID = 10249 }
};

return AssertQuery<Order>(isAsync, od => od.Where(o => orders.Contains(o)),
entryCount: 2);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_with_constant_list_value_type_id(bool isAsync)
{
return AssertQuery<Order>(isAsync, od => od.Where(o => new List<Order>
{
new Order { OrderID = 10248 },
new Order { OrderID = 10249 }
}.Contains(o)),
entryCount: 2);
}

[ConditionalFact]
public virtual void Contains_over_keyless_entity_throws()
{
Expand Down
20 changes: 20 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.ResultOperators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,26 @@ FROM [Customers] AS [c]
WHERE [c].[CustomerID] IN (N'ALFKI', N'ANATR')");
}

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

AssertSql(
@"SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE [o].[OrderID] IN (10248, 10249)");
}

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

AssertSql(
@"SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
FROM [Orders] AS [o]
WHERE [o].[OrderID] IN (10248, 10249)");
}

public override void Contains_over_entityType_with_null_should_rewrite_to_identity_equality()
{
base.Contains_over_entityType_with_null_should_rewrite_to_identity_equality();
Expand Down