Navigation expansion rewrite - first draft

Known limitations:
- GroupBy: not supported at all
- Include tracking/fixup: include method (one that suppose to do fixup and tracking) is just a stub currently - only does fixup one way and doesn't perform tracking at all,
- Various warnings and negative cases: we used to warn/throw particular exceptions for negative cases, those are now different and the messages themselves are not hardcoded and not polished
- include/project collection - correlated collection feature is disabled, so projecting collections will issue N+1 queries. Same goes for include collection.
- async + collections - no special handling of async, projecting/including collection may cause a deadlock

src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs

@@ -41,7 +41,6 @@ public static IServiceCollection AddEntityFrameworkCosmos([NotNull] this IServic
                 .TryAdd<IEntityQueryModelVisitorFactory, CosmosEntityQueryModelVisitorFactory>()
                 .TryAdd<IEntityQueryableExpressionVisitorFactory, CosmosEntityQueryableExpressionVisitorFactory>()
                 .TryAdd<IMemberAccessBindingExpressionVisitorFactory, CosmosMemberAccessBindingExpressionVisitorFactory>()
-                .TryAdd<INavigationRewritingExpressionVisitorFactory, CosmosNavigationRewritingExpressionVisitorFactory>()
                 .TryAdd<ITypeMappingSource, CosmosTypeMappingSource>()
                 .TryAddProviderSpecificServices(
                     b => b

src/EFCore.Relational/Query/ExpressionVisitors/RelationalProjectionExpressionVisitor.cs

@@ -180,8 +180,7 @@ public override Expression Visit(Expression expression)
             // Skip over Include and Correlated Collection methods
             // This is checked first because it should not call base when there is not _targetSelectExpression
             if (expression is MethodCallExpression methodCallExpression
-                && (IncludeCompiler.IsIncludeMethod(methodCallExpression)
-                    || CorrelatedCollectionOptimizingVisitor.IsCorrelatedCollectionMethod(methodCallExpression)))
+                && IncludeCompiler.IsIncludeMethod(methodCallExpression))
             {
                 return expression;
             }
@@ -279,7 +278,10 @@ var sqlExpression
             // We bind with ValueBuffer in GroupByAggregate case straight away
             // Since the expression can be some translation from [g].[Key] which won't bind with MemberAccessBindingEV
             if (!_isGroupAggregate
-                && sqlExpression is ColumnExpression)
+                && sqlExpression is ColumnExpression
+                // if we have null conditional that propagates null, i.e. entity.Key != null ? entity.Property : null
+                // the result is also a ColumnExpression, but for this case we don't want to short-circuit
+                && !(expression is ConditionalExpression))
             {
                 var index = _targetSelectExpression.AddToProjection(sqlExpression);
 

src/EFCore.Relational/Query/ExpressionVisitors/SqlTranslatingExpressionVisitor.cs

@@ -14,6 +14,7 @@
 using Microsoft.EntityFrameworkCore.Query.Expressions.Internal;
 using Microsoft.EntityFrameworkCore.Query.ExpressionTranslators;
 using Microsoft.EntityFrameworkCore.Query.Internal;
+using Microsoft.EntityFrameworkCore.Query.NavigationExpansion;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Utilities;
 using Remotion.Linq;
@@ -1147,11 +1148,8 @@ protected override Expression VisitExtension(Expression expression)
 
                     return new NullableExpression(newAccessOperation);
 
-                case NullSafeEqualExpression nullSafeEqualExpression:
-                    var equalityExpression
-                        = new NullCompensatedExpression(nullSafeEqualExpression.EqualExpression);
-
-                    return Visit(equalityExpression);
+                case CorrelationPredicateExpression correlationPredicateExpression:
+                    return Visit(new NullCompensatedExpression(correlationPredicateExpression.EqualExpression));
 
                 case NullCompensatedExpression nullCompensatedExpression:
                     newOperand = Visit(nullCompensatedExpression.Operand);

src/EFCore.Relational/Query/RelationalQueryModelVisitor.cs

@@ -1246,34 +1246,6 @@ var sqlOrderingExpression
             }
         }
 
-        /// <summary>
-        ///     Determines whether correlated collections (if any) can be optimized.
-        /// </summary>
-        /// <returns>True if optimization is allowed, false otherwise.</returns>
-        protected override bool CanOptimizeCorrelatedCollections()
-        {
-            if (!base.CanOptimizeCorrelatedCollections())
-            {
-                return false;
-            }
-
-            if (RequiresClientEval
-                || RequiresClientFilter
-                || RequiresClientJoin
-                || RequiresClientOrderBy
-                || RequiresClientSelectMany)
-            {
-                return false;
-            }
-
-            var injectParametersFinder
-                = new InjectParametersFindingVisitor(QueryCompilationContext.QueryMethodProvider.InjectParametersMethod);
-
-            injectParametersFinder.Visit(Expression);
-
-            return !injectParametersFinder.InjectParametersFound;
-        }
-
         private class InjectParametersFindingVisitor : ExpressionVisitorBase
         {
             private readonly MethodInfo _injectParametersMethod;
@@ -1403,66 +1375,6 @@ public override void VisitResultOperator(ResultOperatorBase resultOperator, Quer
             }
         }
 
-        private class GroupByPreProcessor : QueryModelVisitorBase
-        {
-            private readonly QueryCompilationContext _queryCompilationContext;
-
-            public GroupByPreProcessor(QueryCompilationContext queryCompilationContext)
-            {
-                _queryCompilationContext = queryCompilationContext;
-            }
-
-            public override void VisitQueryModel(QueryModel queryModel)
-            {
-                queryModel.TransformExpressions(new TransformingQueryModelExpressionVisitor<GroupByPreProcessor>(this).Visit);
-
-                if (queryModel.ResultOperators.Any(o => o is GroupResultOperator)
-                    && _queryCompilationContext.IsIncludeQuery
-                    && !queryModel.ResultOperators.Any(
-                        o => o is SkipResultOperator || o is TakeResultOperator || o is ChoiceResultOperatorBase || o is DistinctResultOperator))
-                {
-                    base.VisitQueryModel(queryModel);
-                }
-            }
-
-            protected override void VisitResultOperators(ObservableCollection<ResultOperatorBase> resultOperators, QueryModel queryModel)
-            {
-                var groupResultOperators = queryModel.ResultOperators.OfType<GroupResultOperator>().ToList();
-                if (groupResultOperators.Count > 0)
-                {
-                    var orderByClause = queryModel.BodyClauses.OfType<OrderByClause>().FirstOrDefault();
-                    if (orderByClause == null)
-                    {
-                        orderByClause = new OrderByClause();
-                        queryModel.BodyClauses.Add(orderByClause);
-                    }
-
-                    var firstGroupResultOperator = groupResultOperators[0];
-
-                    var groupKeys = firstGroupResultOperator.KeySelector is NewExpression compositeGroupKey
-                        ? compositeGroupKey.Arguments.Reverse()
-                        : new[] { firstGroupResultOperator.KeySelector };
-
-                    foreach (var groupKey in groupKeys)
-                    {
-                        orderByClause.Orderings.Insert(0, new Ordering(groupKey, OrderingDirection.Asc));
-                    }
-                }
-            }
-        }
-
-        /// <summary>
-        ///     Pre-processes query model before we rewrite its navigations.
-        /// </summary>
-        /// <param name="queryModel">Query model to process. </param>
-        protected override void OnBeforeNavigationRewrite(QueryModel queryModel)
-        {
-            Check.NotNull(queryModel, nameof(queryModel));
-
-            var groupByPreProcessor = new GroupByPreProcessor(QueryCompilationContext);
-            groupByPreProcessor.VisitQueryModel(queryModel);
-        }
-
         /// <summary>
         ///     Applies optimizations to the query.
         /// </summary>

src/EFCore/EFCore.csproj

@@ -11,6 +11,7 @@ Microsoft.EntityFrameworkCore.DbSet
     <MinClientVersion>3.6</MinClientVersion>
     <AssemblyName>Microsoft.EntityFrameworkCore</AssemblyName>
     <RootNamespace>Microsoft.EntityFrameworkCore</RootNamespace>
+    <NoWarn>$(NoWarn);CS1591</NoWarn>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <CodeAnalysisRuleSet>..\..\EFCore.ruleset</CodeAnalysisRuleSet>
     <LangVersion>8.0</LangVersion>

src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs

@@ -93,7 +93,6 @@ public static readonly IDictionary<Type, ServiceCharacteristics> CoreServices
                 { typeof(IEntityTrackingInfoFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(ITaskBlockingExpressionVisitor), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(IMemberAccessBindingExpressionVisitorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
-                { typeof(INavigationRewritingExpressionVisitorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(IEagerLoadingExpressionVisitorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(IQuerySourceTracingExpressionVisitorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
                 { typeof(IProjectionExpressionVisitorFactory), new ServiceCharacteristics(ServiceLifetime.Singleton) },
@@ -258,7 +257,6 @@ public virtual EntityFrameworkServicesBuilder TryAddCoreServices()
             TryAdd<ITaskBlockingExpressionVisitor, TaskBlockingExpressionVisitor>();
             TryAdd<IEntityResultFindingExpressionVisitorFactory, EntityResultFindingExpressionVisitorFactory>();
             TryAdd<IMemberAccessBindingExpressionVisitorFactory, MemberAccessBindingExpressionVisitorFactory>();
-            TryAdd<INavigationRewritingExpressionVisitorFactory, NavigationRewritingExpressionVisitorFactory>();
             TryAdd<IEagerLoadingExpressionVisitorFactory, EagerLoadingExpressionVisitorFactory>();
             TryAdd<IQuerySourceTracingExpressionVisitorFactory, QuerySourceTracingExpressionVisitorFactory>();
             TryAdd<IRequiresMaterializationExpressionVisitorFactory, RequiresMaterializationExpressionVisitorFactory>();