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)

CR: Andrew
  • Loading branch information
maumar committed Feb 18, 2016
1 parent fe10657 commit c4e4447
Show file tree
Hide file tree
Showing 19 changed files with 1,691 additions and 285 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 class BufferedOffsetEntityShaper<TEntity> : BufferedEntityShaper<TEntity>

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 class UnbufferedOffsetEntityShaper<TEntity> : UnbufferedEntityShaper<TEnt

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 @@ var nullExpression
|| 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

0 comments on commit c4e4447

Please sign in to comment.