Refactor queryable collection support for non-relational providers

Also fixing exception ordering issue.

Fixes dotnet#32505

src/EFCore.Relational/Query/RelationalQueryRootProcessor.cs

@@ -28,7 +28,7 @@ public RelationalQueryRootProcessor(
 
     /// <summary>
     ///     Indicates that a <see cref="ConstantExpression" /> can be converted to a <see cref="InlineQueryRootExpression" />;
-    ///     the latter will end up in <see cref="RelationalQueryableMethodTranslatingExpressionVisitor.VisitInlineQueryRoot" /> for
+    ///     the latter will end up in <see cref="RelationalQueryableMethodTranslatingExpressionVisitor.TranslateInlineQueryRoot" /> for
     ///     translation to a SQL <see cref="ValuesExpression" />.
     /// </summary>
     protected override bool ShouldConvertToInlineQueryRoot(NewArrayExpression newArrayExpression)

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -210,19 +210,6 @@ when entityQueryRootExpression.GetType() == typeof(EntityQueryRootExpression)
                 return new ShapedQueryExpression(selectExpression, shaperExpression);
             }
 
-            case InlineQueryRootExpression inlineQueryRootExpression:
-                return VisitInlineQueryRoot(inlineQueryRootExpression) ?? base.VisitExtension(extensionExpression);
-
-            case ParameterQueryRootExpression parameterQueryRootExpression:
-                var sqlParameterExpression =
-                    _sqlTranslator.Visit(parameterQueryRootExpression.ParameterExpression) as SqlParameterExpression;
-                Check.DebugAssert(sqlParameterExpression is not null, "sqlParameterExpression is not null");
-                return TranslatePrimitiveCollection(
-                        sqlParameterExpression,
-                        property: null,
-                        char.ToLowerInvariant(sqlParameterExpression.Name.First(c => c != '_')).ToString())
-                    ?? base.VisitExtension(extensionExpression);
-
             case JsonQueryExpression jsonQueryExpression:
                 return TransformJsonQueryToTable(jsonQueryExpression) ?? base.VisitExtension(extensionExpression);
 
@@ -264,25 +251,6 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
 
         if (translated == QueryCompilationContext.NotTranslatedExpression)
         {
-            // Attempt to translate access into a primitive collection property (i.e. array column)
-            if (_sqlTranslator.TryTranslatePropertyAccess(methodCallExpression, out var translatedExpression, out var property)
-                && property is IProperty { IsPrimitiveCollection: true } regularProperty
-                && translatedExpression is SqlExpression sqlExpression)
-            {
-                var tableAlias = sqlExpression switch
-                {
-                    ColumnExpression c => c.Name[..1].ToLowerInvariant(),
-                    JsonScalarExpression { Path: [.., { PropertyName: string propertyName }] } => propertyName[..1].ToLowerInvariant(),
-                    _ => "j"
-                };
-
-                if (TranslatePrimitiveCollection(sqlExpression, regularProperty, tableAlias) is
-                    { } primitiveCollectionTranslation)
-                {
-                    return primitiveCollectionTranslation;
-                }
-            }
-
             // For Contains over a collection parameter, if the provider hasn't implemented TranslateCollection (e.g. OPENJSON on SQL
             // Server), we need to fall back to the previous IN translation.
             if (method.IsGenericMethod
@@ -304,13 +272,57 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
         return translated;
     }
 
+    /// <inheritdoc />
+    protected override ShapedQueryExpression? TranslateMemberAccess(Expression source, MemberIdentity member)
+    {
+        // Attempt to translate access into a primitive collection property (i.e. array column)
+        if (_sqlTranslator.TryBindMember(_sqlTranslator.Visit(source), member, out var translatedExpression, out var property)
+            && property is IProperty { IsPrimitiveCollection: true } regularProperty
+            && translatedExpression is SqlExpression sqlExpression)
+        {
+            var tableAlias = sqlExpression switch
+            {
+                ColumnExpression c => c.Name[..1].ToLowerInvariant(),
+                JsonScalarExpression { Path: [.., { PropertyName: string propertyName }] } => propertyName[..1].ToLowerInvariant(),
+                _ => "j"
+            };
+
+            if (TranslatePrimitiveCollection(sqlExpression, regularProperty, tableAlias) is
+                { } primitiveCollectionTranslation)
+            {
+                return primitiveCollectionTranslation;
+            }
+        }
+
+        if (_sqlTranslator.TranslationErrorDetails != null)
+        {
+            AddTranslationErrorDetails(_sqlTranslator.TranslationErrorDetails);
+        }
+
+        return null;
+    }
+
+    /// <inheritdoc />
+    protected override ShapedQueryExpression? TranslateParameterQueryRoot(ParameterQueryRootExpression parameterQueryRootExpression)
+    {
+        var sqlParameterExpression =
+            _sqlTranslator.Visit(parameterQueryRootExpression.ParameterExpression) as SqlParameterExpression;
+
+        Check.DebugAssert(sqlParameterExpression is not null, "sqlParameterExpression is not null");
+
+        return TranslatePrimitiveCollection(
+            sqlParameterExpression,
+            property: null,
+            char.ToLowerInvariant(sqlParameterExpression.Name.First(c => c != '_')).ToString());
+    }
+
     /// <summary>
     ///     Translates a parameter or column collection of primitive values. Providers can override this to translate e.g. int[] columns or
     ///     parameters to a queryable table (OPENJSON on SQL Server, unnest on PostgreSQL...). The default implementation always returns
     ///     <see langword="null" /> (no translation).
     /// </summary>
     /// <remarks>
-    ///     Inline collections aren't passed to this method; see <see cref="VisitInlineQueryRoot" /> for the translation of inline
+    ///     Inline collections aren't passed to this method; see <see cref="TranslateInlineQueryRoot" /> for the translation of inline
     ///     collections.
     /// </remarks>
     /// <param name="sqlExpression">The expression to try to translate as a primitive collection expression.</param>
@@ -346,7 +358,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
     /// </summary>
     /// <param name="inlineQueryRootExpression">The inline collection to be translated.</param>
     /// <returns>A queryable SQL VALUES expression.</returns>
-    protected virtual ShapedQueryExpression? VisitInlineQueryRoot(InlineQueryRootExpression inlineQueryRootExpression)
+    protected override ShapedQueryExpression? TranslateInlineQueryRoot(InlineQueryRootExpression inlineQueryRootExpression)
     {
         var elementType = inlineQueryRootExpression.ElementType;
 

src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs

@@ -760,38 +760,6 @@ protected override Expression VisitMemberInit(MemberInitExpression memberInitExp
             ? sqlConstantExpression
             : QueryCompilationContext.NotTranslatedExpression;
 
-    /// <summary>
-    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-    ///     any release. You should only use it directly in your code with extreme caution and knowing that
-    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-    /// </summary>
-    [EntityFrameworkInternal]
-    public virtual bool TryTranslatePropertyAccess(
-        Expression expression,
-        [NotNullWhen(true)] out Expression? translatedExpression,
-        [NotNullWhen(true)] out IPropertyBase? property)
-    {
-        if (expression is MethodCallExpression methodCallExpression)
-        {
-            if (methodCallExpression.TryGetEFPropertyArguments(out var source, out var propertyName)
-                && TryBindMember(Visit(source), MemberIdentity.Create(propertyName), out translatedExpression, out property))
-            {
-                return true;
-            }
-
-            if (methodCallExpression.TryGetIndexerArguments(_model, out source, out propertyName)
-                && TryBindMember(Visit(source), MemberIdentity.Create(propertyName), out translatedExpression, out property))
-            {
-                return true;
-            }
-        }
-
-        translatedExpression = null;
-        property = null;
-        return false;
-    }
-
     /// <inheritdoc />
     protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
     {
@@ -1258,7 +1226,13 @@ private bool TryBindMember(
         [NotNullWhen(true)] out Expression? expression)
         => TryBindMember(source, member, out expression, out _);
 
-    private bool TryBindMember(
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public virtual bool TryBindMember(
         Expression? source,
         MemberIdentity member,
         [NotNullWhen(true)] out Expression? expression,