Skip to content

Commit

Permalink
Fix to #7290 - Query compilation error or data corruption for queries…
Browse files Browse the repository at this point in the history
… with GroupJoin wrapped in a subquery when the subset of columns is being projected

Problem was that we were incorrectly reasoning about which query sources need to be materialized in the GroupJoin scenario. Previously, when we encountered GroupJoin, we marked its main from clause and the joined clause for materialization, but we would not do that recursively. This causes problems if the GroupJoin (e.g. optional navigation) is in a subquery - we would not mark it for materialization. This would lead to compilation errors in some cases, and in others (when there was a client-evaluation component) to data corruption.

Fix is to use a QueryModelVisitor to walk the entire tree which allows all necessary query sources related to GroupJoin to be marked for materialization. This incurs a performance hit for some scenarios (groupjoin/optional navigation inside a subquery) because now we bring more columns from the database.

Second part of the fix is that in LiftSubquery, when we update query sources, we would sometimes try to rewrite EntityShapers into ValueBufferShapers (if the lifted query itself is not marked for materialization). This however is not safe to do in case of GroupJoin. Fix is to only perform this conversion shapers inside ShapedQuery method.
  • Loading branch information
maumar committed Jan 30, 2017
1 parent b359086 commit deed422
Show file tree
Hide file tree
Showing 9 changed files with 785 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,7 @@ private sealed class QuerySourceUpdater : ExpressionVisitorBase
private readonly RelationalQueryCompilationContext _relationalQueryCompilationContext;
private readonly ILinqOperatorProvider _linqOperatorProvider;
private readonly SelectExpression _selectExpression;
private bool _insideShapedQueryMethod;

public QuerySourceUpdater(
IQuerySource querySource,
Expand All @@ -1011,8 +1012,9 @@ in _relationalQueryCompilationContext.QueryAnnotations
queryAnnotation.QuerySource = _querySource;
}

if (!_relationalQueryCompilationContext.QuerySourceRequiresMaterialization(_querySource)
&& shaper is EntityShaper)
if (_insideShapedQueryMethod
&& shaper is EntityShaper
&& !_relationalQueryCompilationContext.QuerySourceRequiresMaterialization(_querySource))
{
return Expression.Constant(new ValueBufferShaper(_querySource));
}
Expand All @@ -1027,12 +1029,14 @@ in _relationalQueryCompilationContext.QueryAnnotations

protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
{
_insideShapedQueryMethod = methodCallExpression.Method.MethodIsClosedFormOf(
_relationalQueryCompilationContext.QueryMethodProvider.ShapedQueryMethod);

var arguments = VisitAndConvert(methodCallExpression.Arguments, "VisitMethodCall");

if (arguments != methodCallExpression.Arguments)
{
if (methodCallExpression.Method.MethodIsClosedFormOf(
_relationalQueryCompilationContext.QueryMethodProvider.ShapedQueryMethod))
if (_insideShapedQueryMethod)
{
return Expression.Call(
_relationalQueryCompilationContext.QueryMethodProvider.ShapedQueryMethod
Expand Down
Loading

0 comments on commit deed422

Please sign in to comment.