Restrain ability to skip member expressions

Expressions' visit can be skipped only in order by or group by and for
less expression types

Author: alex-kulakov

Orm/Xtensive.Orm/Orm/Linq/ExpressionExtensions.cs

@@ -1,6 +1,6 @@
-// Copyright (C) 2003-2010 Xtensive LLC.
-// All rights reserved.
-// For conditions of distribution and use, see license.
+// Copyright (C) 2008-2024 Xtensive LLC.
+// This code is distributed under MIT license terms.
+// See the License.txt file in the project root for more information.
 // Created by: Alexey Kochetov
 // Created:    2008.12.02
 
@@ -89,6 +89,11 @@ private static bool IsForeignQuery(Expression expression)
       return false;
     }
 
+    public static bool IsExtendedExpression(this Expression expression)
+    {
+      return (ExtendedExpressionType) expression.StripMarkers().NodeType >= ExtendedExpressionType.Projection;
+    }
+
     public static bool IsItemProjector(this Expression expression)
     {
       expression = expression.StripMarkers();
@@ -151,12 +156,16 @@ public static Type GetGroupingKeyType(this Expression expression)
 
     public static Expression StripMarkers(this Expression e)
     {
-      var ee = e as ExtendedExpression;
-      if (ee!=null && ee.ExtendedType==ExtendedExpressionType.Marker) {
-        var marker = (MarkerExpression) ee;
-        return marker.Target;
+      if ((ExtendedExpressionType)e.NodeType==ExtendedExpressionType.Marker) {
+        return ((MarkerExpression) e).Target;
       }
       return e;
+      //var ee = e as ExtendedExpression;
+      //if (ee!=null && ee.ExtendedType==ExtendedExpressionType.Marker) {
+      //  var marker = (MarkerExpression) ee;
+      //  return marker.Target;
+      //}
+      //return e;
     }
 
     public static bool IsMarker(this Expression e)

Orm/Xtensive.Orm/Orm/Linq/Translator.Expressions.cs

@@ -1343,26 +1343,41 @@ private Expression GetMember(Expression expression, MemberInfo member, Expressio
         if (memberIndex < 0)
           throw new InvalidOperationException(string.Format(Strings.ExCouldNotGetMemberXFromExpression, member));
         var argument = newExpression.Arguments[memberIndex];
-        var currentLambda = state.CurrentLambda;
-        if (!(currentLambda is null) && context.Bindings.TryGetValue(currentLambda.Parameters[0], out var projection)) {
-          // Here, we try to detect whether the expression has already visited, in such cases
-          // re-visiting it may cause issues. For instance, .OrderBy(x=>x.SomeField) translation
-          // gets projection of already visited source and substitutes parameter 'x' access with 
-          // that projection. For now TranslatorState.ShouldOmitConvertToObject == true, is a marker
-          // of OrderBy visiting, because it is the only place that sets 'true' to the property,
-          // but we can't rely on it for future-proof solution.
-
-          // Knowing that parameter-to-projection binding happens when expression has visited,
-          // we assume this check is enough for certain cases, probably not for all of them.
-          // We can't deny visiting of the argument for all the cases because of chance to break
-          // some cases we could not imagine at the time of changes
-          switch (argument.NodeType) {
-            case ExpressionType.TypeAs:
-            case ExpressionType.Convert:
-            case ExpressionType.ConvertChecked:
-              if (argument.Type == typeof(object) && state.ShouldOmitConvertToObject)
-                argument = argument.StripCasts();
-              break;
+
+        if (state.GroupingKey || state.OrderingKey) {
+          var requireVisit = !argument.IsExtendedExpression() && argument.NodeType switch {
+            ExpressionType.New => true,
+            ExpressionType.Call => true,
+            ExpressionType.Coalesce => true,
+            ExpressionType.Conditional => true,
+            _ => false
+          };
+
+          var currentLambda = state.CurrentLambda;
+          if (!(currentLambda is null) && context.Bindings.TryGetValue(currentLambda.Parameters[0], out var projection) 
+            && !requireVisit) {
+            // Here, we try to detect whether the expression has already visited, in such cases
+            // re-visiting it may cause issues. For instance, .OrderBy(x=>x.SomeField) translation
+            // gets projection of already visited source and substitutes parameter 'x' access with 
+            // that projection. For now TranslatorState.ShouldOmitConvertToObject == true, is a marker
+            // of OrderBy visiting, because it is the only place that sets 'true' to the property,
+            // but we can't rely on it for future-proof solution.
+
+            // Knowing that parameter-to-projection binding happens when expression has visited,
+            // we assume this check is enough for certain cases, probably not for all of them.
+            // We can't deny visiting of the argument for all the cases because of chance to break
+            // some cases we could not imagine at the time of changes
+            switch (argument.NodeType) {
+              case ExpressionType.TypeAs:
+              case ExpressionType.Convert:
+              case ExpressionType.ConvertChecked:
+                if (argument.Type == typeof(object) && state.ShouldOmitConvertToObject)
+                  argument = argument.StripCasts();
+                break;
+            }
+          }
+          else {
+            argument = Visit(argument);
           }
         }
         else {

Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs

@@ -988,6 +988,7 @@ private Expression VisitSort(Expression expression)
         using (state.CreateScope()) {
           state.ShouldOmitConvertToObject = true;
           state.CalculateExpressions = true;
+          state.OrderingKey = true;
           var orderByProjector = (ItemProjectorExpression) VisitLambda(sortExpression);
           var columns = orderByProjector
             .GetColumns(ColumnExtractionModes.TreatEntityAsKey | ColumnExtractionModes.Distinct);

Orm/Xtensive.Orm/Orm/Linq/TranslatorState.cs

@@ -25,7 +25,8 @@ private enum TranslatorStateFlags
       ShouldOmitConvertToObject = 1 << 6,
       RequestCalculateExpressions = 1 << 7,
       RequestCalculateExpressionsOnce = 1 << 8,
-      SkipNullableColumnsDetectionInGroupBy = 1 << 9
+      SkipNullableColumnsDetectionInGroupBy = 1 << 9,
+      IsOrderingKey = 1 << 10,
     }
 
     internal readonly ref struct TranslatorScope
@@ -105,6 +106,14 @@ public bool GroupingKey
         : flags & ~TranslatorStateFlags.IsGroupingKey;
     }
 
+    public bool OrderingKey
+    {
+      get => (flags & TranslatorStateFlags.IsOrderingKey) != 0;
+      set => flags = value
+        ? flags | TranslatorStateFlags.IsOrderingKey
+        : flags & ~TranslatorStateFlags.IsOrderingKey;
+    }
+
     public bool IsTailMethod
     {
       get => (flags & TranslatorStateFlags.IsTailMethod) != 0;