Skip to content

Commit

Permalink
Query: Generate predicate correctly when expanding owned collections
Browse files Browse the repository at this point in the history 
Resolves #23130

Some additional tests changed because it ended up causing client eval in the middle due to another level of OwnsMany
  • Loading branch information
@smitpatel
smitpatel committed Nov 17, 2020
1 parent 593d967 commit c952f4d
Show file tree
Hide file tree
Showing 6 changed files with 478 additions and 136 deletions.
53 changes: 43 additions & 10 deletions src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1282,9 +1282,13 @@ internal Expression ExpandWeakEntities(InMemoryQueryExpression queryExpression,

private sealed class WeakEntityExpandingExpressionVisitor : ExpressionVisitor
{
private InMemoryQueryExpression _queryExpression;
private static readonly MethodInfo _objectEqualsMethodInfo
= typeof(object).GetRuntimeMethod(nameof(object.Equals), new[] { typeof(object), typeof(object) });

private readonly InMemoryExpressionTranslatingExpressionVisitor _expressionTranslator;

private InMemoryQueryExpression _queryExpression;

public WeakEntityExpandingExpressionVisitor(InMemoryExpressionTranslatingExpressionVisitor expressionTranslator)
{
_expressionTranslator = expressionTranslator;
Expand Down Expand Up @@ -1402,15 +1406,39 @@ private Expression TryExpand(Expression source, MemberIdentity member)
: foreignKey.Properties,
makeNullable);

var outerKeyFirstProperty = outerKey is NewExpression newExpression
? ((UnaryExpression)((NewArrayExpression)newExpression.Arguments[0]).Expressions[0]).Operand
: outerKey;

var predicate = outerKeyFirstProperty.Type.IsNullableType()
? Expression.AndAlso(
Expression.NotEqual(outerKeyFirstProperty, Expression.Constant(null, outerKeyFirstProperty.Type)),
Expression.Equal(outerKey, innerKey))
: Expression.Equal(outerKey, innerKey);
Expression predicate = null;
if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23130", out var isEnabled) && isEnabled)
{
var outerKeyFirstProperty = outerKey is NewExpression newExpression
? ((UnaryExpression)((NewArrayExpression)newExpression.Arguments[0]).Expressions[0]).Operand
: outerKey;

predicate = outerKeyFirstProperty.Type.IsNullableType()
? Expression.AndAlso(
Expression.NotEqual(outerKeyFirstProperty, Expression.Constant(null, outerKeyFirstProperty.Type)),
Expression.Equal(outerKey, innerKey))
: Expression.Equal(outerKey, innerKey);
}
else
{
var keyComparison = Expression.Call(_objectEqualsMethodInfo, AddConvertToObject(outerKey), AddConvertToObject(innerKey));

predicate = makeNullable
? Expression.AndAlso(
outerKey is NewArrayExpression newArrayExpression
? newArrayExpression.Expressions
.Select(
e =>
{
var left = (e as UnaryExpression)?.Operand ?? e;
return Expression.NotEqual(left, Expression.Constant(null, left.Type));
})
.Aggregate((l, r) => Expression.AndAlso(l, r))
: Expression.NotEqual(outerKey, Expression.Constant(null, outerKey.Type)),
keyComparison)
: (Expression)keyComparison;
}

var correlationPredicate = _expressionTranslator.Translate(predicate);
innerQueryExpression.UpdateServerQueryExpression(
Expand Down Expand Up @@ -1460,6 +1488,11 @@ ProjectionBindingExpression projectionBindingExpression

return innerShaper;
}

private static Expression AddConvertToObject(Expression expression)
=> expression.Type.IsValueType
? Expression.Convert(expression, typeof(object))
: expression;
}

private ShapedQueryExpression TranslateScalarAggregate(
Expand Down
53 changes: 43 additions & 10 deletions src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1220,10 +1220,14 @@ internal Expression ExpandWeakEntities(SelectExpression selectExpression, Expres

private sealed class WeakEntityExpandingExpressionVisitor : ExpressionVisitor
{
private SelectExpression _selectExpression;
private static readonly MethodInfo _objectEqualsMethodInfo
= typeof(object).GetRuntimeMethod(nameof(object.Equals), new[] { typeof(object), typeof(object) });

private readonly RelationalSqlTranslatingExpressionVisitor _sqlTranslator;
private readonly ISqlExpressionFactory _sqlExpressionFactory;

private SelectExpression _selectExpression;

public WeakEntityExpandingExpressionVisitor(
RelationalSqlTranslatingExpressionVisitor sqlTranslator,
ISqlExpressionFactory sqlExpressionFactory)
Expand Down Expand Up @@ -1347,15 +1351,39 @@ private Expression TryExpand(Expression source, MemberIdentity member)
: foreignKey.Properties,
makeNullable);

var outerKeyFirstProperty = outerKey is NewExpression newExpression
? ((UnaryExpression)((NewArrayExpression)newExpression.Arguments[0]).Expressions[0]).Operand
: outerKey;

var predicate = outerKeyFirstProperty.Type.IsNullableType()
? Expression.AndAlso(
Expression.NotEqual(outerKeyFirstProperty, Expression.Constant(null, outerKeyFirstProperty.Type)),
Expression.Equal(outerKey, innerKey))
: Expression.Equal(outerKey, innerKey);
Expression predicate = null;
if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23130", out var isEnabled) && isEnabled)
{
var outerKeyFirstProperty = outerKey is NewExpression newExpression
? ((UnaryExpression)((NewArrayExpression)newExpression.Arguments[0]).Expressions[0]).Operand
: outerKey;

predicate = outerKeyFirstProperty.Type.IsNullableType()
? Expression.AndAlso(
Expression.NotEqual(outerKeyFirstProperty, Expression.Constant(null, outerKeyFirstProperty.Type)),
Expression.Equal(outerKey, innerKey))
: Expression.Equal(outerKey, innerKey);
}
else
{
var keyComparison = Expression.Call(_objectEqualsMethodInfo, AddConvertToObject(outerKey), AddConvertToObject(innerKey));

predicate = makeNullable
? Expression.AndAlso(
outerKey is NewArrayExpression newArrayExpression
? newArrayExpression.Expressions
.Select(
e =>
{
var left = (e as UnaryExpression)?.Operand ?? e;
return Expression.NotEqual(left, Expression.Constant(null, left.Type));
})
.Aggregate((l, r) => Expression.AndAlso(l, r))
: Expression.NotEqual(outerKey, Expression.Constant(null, outerKey.Type)),
keyComparison)
: (Expression)keyComparison;
}

var correlationPredicate = Expression.Lambda(predicate, correlationPredicateParameter);

Expand Down Expand Up @@ -1446,6 +1474,11 @@ private Expression TryExpand(Expression source, MemberIdentity member)
return innerShaper;
}

private static Expression AddConvertToObject(Expression expression)
=> expression.Type.IsValueType
? Expression.Convert(expression, typeof(object))
: expression;

private static IDictionary<IProperty, ColumnExpression> GetPropertyExpressionFromSameTable(
IEntityType entityType,
ITableBase table,
Expand Down
33 changes: 33 additions & 0 deletions test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,39 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
OrderDate = Convert.ToDateTime("2016-04-25 19:23:56")
}
);
ob.OwnsMany(e => e.Details, odb =>
{
odb.HasData(
new
{
Id = -100,
OrderId = -10,
OrderClientId = 1,
Detail = "Discounted Order"
},
new
{
Id = -101,
OrderId = -10,
OrderClientId = 1,
Detail = "Full Price Order"
},
new
{
Id = -200,
OrderId = -20,
OrderClientId = 2,
Detail = "Internal Order"
},
new
{
Id = -300,
OrderId = -30,
OrderClientId = 3,
Detail = "Bulk Order"
});
});
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Relational.Specification.Tests/Query/OwnedQueryRelationalTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public virtual Task Unmapped_property_projection_loads_owned_navigations_split(b
return AssertQuery(
async,
ss => ss.Set<OwnedPerson>().Where(e => e.Id == 1).AsTracking().Select(e => new { e.ReadOnlyProperty }).AsSplitQuery(),
entryCount: 5);
entryCount: 7);
}

[ConditionalTheory]
Expand Down
Loading

0 comments on commit c952f4d

Please sign in to comment.