Skip to content

Commit

Permalink
Initial support for optional navigation translation into LEFT OUTER J…
Browse files Browse the repository at this point in the history
…OIN.

Currently we translate all navigations into INNER JOINs, which may produce invalid results for some queries.

Fix is to translate optional navigations into LEFT OUTER JOIN. In order to do that we convert optional navigation into SelectMany-GroupJoin-DefaultIfEmpty expression, that translates to LOJ.
We also need to add null checks in various places because now the intermittent results can be null.

e.g.
from c in ctx.Customers
where c.Detail.Name == "Foo"
select c

will get translated into:

from c in ctx.Customers
join d in ctx.Detail on c.Id equals d.CustomerId into grouping
from d in grouping.DefaultIfEmpty()
where (d != null ? d.Name : null) == "Foo"
select c;

Also fixed a number of bugs uncovered by this change, due to generating significantly more complex queries in some cases.

Known issues:
- DefaultIfEmpty() cannot be translated into SQL so everything happening after it is computed on the client (filters, projections, nested navigations) (#4539),
- Optional navigations don't work with Include (#4589),
- Optional navigations don't work with some queries involving SelectMany operator (#4539),
- Optional navigations don't work for soem complex queries involving subqueries and/or navigation inside inner key selector of a Join statement (#4547)
  • Loading branch information
maumar committed Feb 18, 2016
1 parent 8e00ac5 commit 5284d5a
Show file tree
Hide file tree
Showing 19 changed files with 1,698 additions and 288 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,10 @@ public override EntityShaper WithOffset(int offset)
Key,
Materializer)
.SetOffset(offset);

public override string ToString()
{
return "BufferedEntityShaper<" + typeof(TEntity).Name + ">";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,10 @@ public BufferedOffsetEntityShaper(

public override TEntity Shape(QueryContext queryContext, ValueBuffer valueBuffer)
=> base.Shape(queryContext, valueBuffer.WithOffset(ValueBufferOffset));

public override string ToString()
{
return "BufferedOffsetEntityShaper<" + typeof(TEntity).Name + ">";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal
{
public class IsNullExpressionBuildingVisitor : RelinqExpressionVisitor
{
private bool _nullConstantAdded = false;

public virtual Expression ResultExpression { get; private set; }

protected override Expression VisitConstant(ConstantExpression node)
{
if (node.Value == null)
if (node.Value == null && !_nullConstantAdded)
{
AddToResult(node);
AddToResult(new IsNullExpression(node));
_nullConstantAdded = true;
}

return node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public virtual Expression Flatten([NotNull] MethodCallExpression methodCallExpre
if (methodCallExpression.Method.MethodIsClosedFormOf(_operatorToFlatten))
{
var outerShapedQuery = (MethodCallExpression)methodCallExpression.Arguments[0];

var outerShaper = (Shaper)((ConstantExpression)outerShapedQuery.Arguments[2]).Value;

var innerLambda
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,10 @@ public override EntityShaper WithOffset(int offset)
Key,
Materializer)
.SetOffset(offset);

public override string ToString()
{
return "UnbufferedEntityShaper<" + typeof(TEntity).Name + ">";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,10 @@ public UnbufferedOffsetEntityShaper(

public override TEntity Shape(QueryContext queryContext, ValueBuffer valueBuffer)
=> base.Shape(queryContext, valueBuffer.WithOffset(ValueBufferOffset));

public override string ToString()
{
return "UnbufferedOffsetEntityShaper<" + typeof(TEntity).Name + ">";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ protected override Expression VisitConditional(ConditionalExpression expression)
{
Check.NotNull(expression, nameof(expression));

var nullCheckRemoved = TryRemoveNullCheck(expression);
if (nullCheckRemoved != null)
{
return Visit(nullCheckRemoved);
}

var test = Visit(expression.Test);
var ifTrue = Visit(expression.IfTrue);
var ifFalse = Visit(expression.IfFalse);
Expand All @@ -186,6 +192,38 @@ protected override Expression VisitConditional(ConditionalExpression expression)
return null;
}

private Expression TryRemoveNullCheck(ConditionalExpression node)
{
var binaryTest = node.Test as BinaryExpression;
if (binaryTest == null || binaryTest.NodeType != ExpressionType.NotEqual)
{
return null;
}

var rightConstant = binaryTest.Right as ConstantExpression;
if (rightConstant == null || rightConstant.Value != null)
{
return null;
}

var ifFalseConstant = node.IfFalse as ConstantExpression;
if (ifFalseConstant == null || ifFalseConstant.Value != null)
{
return null;
}

var ifTrueMemberExpression = node.IfTrue.RemoveConvert() as MemberExpression;
var correctMemberExpression = ifTrueMemberExpression != null
&& ifTrueMemberExpression.Expression == binaryTest.Left;

var ifTruePropertyMethodCallExpression = node.IfTrue.RemoveConvert() as MethodCallExpression;
var correctPropertyMethodCallExpression = ifTruePropertyMethodCallExpression != null
&& EntityQueryModelVisitor.IsPropertyMethod(ifTruePropertyMethodCallExpression.Method)
&& ifTruePropertyMethodCallExpression.Arguments[0] == binaryTest.Left;

return correctMemberExpression || correctPropertyMethodCallExpression ? node.IfTrue : null;
}

private static Expression UnfoldStructuralComparison(ExpressionType expressionType, Expression expression)
{
var binaryExpression = expression as BinaryExpression;
Expand Down Expand Up @@ -242,7 +280,7 @@ private Expression ProcessComparisonExpression(BinaryExpression binaryExpression
return null;
}

var nullExpression
var nullExpression
= TransformNullComparison(leftExpression, rightExpression, binaryExpression.NodeType);

return nullExpression
Expand All @@ -256,8 +294,8 @@ private static Expression TransformNullComparison(
|| expressionType == ExpressionType.NotEqual)
{
var constantExpression
= right as ConstantExpression
?? left as ConstantExpression;
= right.RemoveConvert() as ConstantExpression
?? left.RemoveConvert() as ConstantExpression;

if (constantExpression != null
&& constantExpression.Value == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,24 @@ protected override Expression VisitUnary(UnaryExpression node)
Expression.Constant(false, typeof(bool)));
}

if (!_insideConditionalTest
&& node.NodeType == ExpressionType.Not
&& node.Operand is IsNullExpression)
{
return Expression.Condition(
node.Operand,
Expression.Constant(false, typeof(bool)),
Expression.Constant(true, typeof(bool)));
}

if (!_insideConditionalTest && node.Operand is IsNullExpression)
{
return Expression.Condition(
node,
Expression.Constant(true, typeof(bool)),
Expression.Constant(false, typeof(bool)));
}

return base.VisitUnary(node);
}

Expand Down
Loading

0 comments on commit 5284d5a

Please sign in to comment.