API review fixes

src/EFCore.Cosmos/Query/Internal/ISqlExpressionFactory.cs

@@ -50,7 +50,7 @@ public interface ISqlExpressionFactory
         SqlExpression left,
         SqlExpression right,
         CoreTypeMapping? typeMapping,
-        SqlExpression? existingExpr = null);
+        SqlExpression? existingExpression = null);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs

@@ -320,14 +320,14 @@ var t when t.TryGetSequenceType() != typeof(object) => t,
         SqlExpression left,
         SqlExpression right,
         CoreTypeMapping? typeMapping,
-        SqlExpression? existingExpr = null)
+        SqlExpression? existingExpression = null)
     {
         switch (operatorType)
         {
             case ExpressionType.AndAlso:
-                return ApplyTypeMapping(AndAlso(left, right, existingExpr), typeMapping);
+                return ApplyTypeMapping(AndAlso(left, right, existingExpression), typeMapping);
             case ExpressionType.OrElse:
-                return ApplyTypeMapping(OrElse(left, right, existingExpr), typeMapping);
+                return ApplyTypeMapping(OrElse(left, right, existingExpression), typeMapping);
         }
 
         if (!SqlBinaryExpression.IsValidOperator(operatorType))
@@ -424,7 +424,7 @@ public virtual SqlExpression LessThanOrEqual(SqlExpression left, SqlExpression r
     public virtual SqlExpression AndAlso(SqlExpression left, SqlExpression right)
         => MakeBinary(ExpressionType.AndAlso, left, right, null)!;
 
-    private SqlExpression AndAlso(SqlExpression left, SqlExpression right, SqlExpression? existingExpr)
+    private SqlExpression AndAlso(SqlExpression left, SqlExpression right, SqlExpression? existingExpression)
     {
         // false && x -> false
         // x && true -> x
@@ -450,11 +450,11 @@ private SqlExpression AndAlso(SqlExpression left, SqlExpression right, SqlExpres
             // the case in which left and right are the same expression is handled above
             return Constant(false);
         }
-        if (existingExpr is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } binaryExpr
+        if (existingExpression is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } binaryExpr
             && left == binaryExpr.Left
             && right == binaryExpr.Right)
         {
-            return existingExpr;
+            return existingExpression;
         }
 
         return new SqlBinaryExpression(ExpressionType.AndAlso, left, right, typeof(bool), null);
@@ -469,7 +469,7 @@ private SqlExpression AndAlso(SqlExpression left, SqlExpression right, SqlExpres
     public virtual SqlExpression OrElse(SqlExpression left, SqlExpression right)
         => MakeBinary(ExpressionType.OrElse, left, right, null)!;
 
-    private SqlExpression OrElse(SqlExpression left, SqlExpression right, SqlExpression? existingExpr)
+    private SqlExpression OrElse(SqlExpression left, SqlExpression right, SqlExpression? existingExpression)
     {
         // true || x -> true
         // x || false -> x
@@ -496,11 +496,11 @@ private SqlExpression OrElse(SqlExpression left, SqlExpression right, SqlExpress
             // the case in which left and right are the same expression is handled above
             return Constant(true);
         }
-        if (existingExpr is SqlBinaryExpression { OperatorType: ExpressionType.OrElse } binaryExpr
+        if (existingExpression is SqlBinaryExpression { OperatorType: ExpressionType.OrElse } binaryExpr
             && left == binaryExpr.Left
             && right == binaryExpr.Right)
         {
-            return existingExpr;
+            return existingExpression;
         }
 
         return new SqlBinaryExpression(ExpressionType.OrElse, left, right, typeof(bool), null);

src/EFCore.Design/Design/DesignTimeServiceCollectionExtensions.cs

@@ -3,7 +3,6 @@
 
 using Microsoft.EntityFrameworkCore.Design.Internal;
 using Microsoft.EntityFrameworkCore.Migrations.Internal;
-using Microsoft.EntityFrameworkCore.Query.Design;
 using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Scaffolding.Internal;
 

src/EFCore.Design/Design/Internal/DbContextOperations.cs

@@ -12,7 +12,6 @@
 using Microsoft.EntityFrameworkCore.Infrastructure.Internal;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
-using Microsoft.EntityFrameworkCore.Query.Design;
 using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Scaffolding.Internal;
 

src/EFCore.Design/Query/Design/IPrecompiledQueryCodeGenerator.cs → src/EFCore.Design/Query/IPrecompiledQueryCodeGenerator.cs

@@ -7,7 +7,7 @@
 using Microsoft.EntityFrameworkCore.Design.Internal;
 using static Microsoft.EntityFrameworkCore.Query.Internal.PrecompiledQueryCodeGenerator;
 
-namespace Microsoft.EntityFrameworkCore.Migrations.Design;
+namespace Microsoft.EntityFrameworkCore.Query;
 
 /// <summary>
 ///     Used to generate code for precompiled queries.

src/EFCore.Design/Query/Design/IPrecompiledQueryCodeGeneratorSelector.cs → src/EFCore.Design/Query/IPrecompiledQueryCodeGeneratorSelector.cs

@@ -3,7 +3,7 @@
 
 using System.Diagnostics.CodeAnalysis;
 
-namespace Microsoft.EntityFrameworkCore.Query.Design;
+namespace Microsoft.EntityFrameworkCore.Query;
 
 /// <summary>
 ///     Selects an <see cref="IPrecompiledQueryCodeGenerator" /> service for a given programming language.

src/EFCore.Design/Query/Internal/PrecompiledQueryCodeGeneratorSelector.cs

@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.EntityFrameworkCore.Design.Internal;
-using Microsoft.EntityFrameworkCore.Query.Design;
 
 namespace Microsoft.EntityFrameworkCore.Query.Internal;
 

src/EFCore.Relational/Query/ISqlExpressionFactory.cs

@@ -41,14 +41,14 @@ public interface ISqlExpressionFactory
     /// <param name="operand">A <see cref="SqlExpression" /> to apply unary operator on.</param>
     /// <param name="type">The type of the created expression.</param>
     /// <param name="typeMapping">A type mapping to be assigned to the created expression.</param>
-    /// <param name="existingExpr">An optional expression that can be re-used if it matches the new expression.</param>
+    /// <param name="existingExpression">An optional expression that can be re-used if it matches the new expression.</param>
     /// <returns>A <see cref="SqlExpression" /> with the given arguments.</returns>
     SqlExpression? MakeUnary(
         ExpressionType operatorType,
         SqlExpression operand,
         Type type,
         RelationalTypeMapping? typeMapping = null,
-        SqlExpression? existingExpr = null);
+        SqlExpression? existingExpression = null);
 
     /// <summary>
     ///     Creates a new <see cref="SqlExpression" /> with the given arguments.
@@ -57,14 +57,14 @@ public interface ISqlExpressionFactory
     /// <param name="left">The left operand of binary operation.</param>
     /// <param name="right">The right operand of binary operation.</param>
     /// <param name="typeMapping">A type mapping to be assigned to the created expression.</param>
-    /// <param name="existingExpr">An optional expression that can be re-used if it matches the new expression.</param>
+    /// <param name="existingExpression">An optional expression that can be re-used if it matches the new expression.</param>
     /// <returns>A <see cref="SqlExpression" /> with the given arguments.</returns>
     SqlExpression? MakeBinary(
         ExpressionType operatorType,
         SqlExpression left,
         SqlExpression right,
         RelationalTypeMapping? typeMapping,
-        SqlExpression? existingExpr = null);
+        SqlExpression? existingExpression = null);
 
     // Comparison
     /// <summary>
@@ -250,13 +250,13 @@ public interface ISqlExpressionFactory
     /// <param name="operand">An expression to compare with <see cref="CaseWhenClause.Test" /> in <paramref name="whenClauses" />.</param>
     /// <param name="whenClauses">A list of <see cref="CaseWhenClause" /> to compare or evaluate and get result from.</param>
     /// <param name="elseResult">A value to return if no <paramref name="whenClauses" /> matches, if any.</param>
-    /// <param name="existingExpr">An optional expression that can be re-used if it matches the new expression.</param>
+    /// <param name="existingExpression">An optional expression that can be re-used if it matches the new expression.</param>
     /// <returns>An expression representing a CASE statement in a SQL tree.</returns>
     SqlExpression Case(
         SqlExpression? operand,
         IReadOnlyList<CaseWhenClause> whenClauses,
         SqlExpression? elseResult,
-        SqlExpression? existingExpr = null);
+        SqlExpression? existingExpression = null);
 
     /// <summary>
     ///     Creates a new <see cref="CaseExpression" /> which represent a CASE statement in a SQL tree.

src/EFCore.Relational/Query/RelationalTypeMappingPostprocessor.cs

@@ -10,47 +10,35 @@ namespace Microsoft.EntityFrameworkCore.Query;
 ///     A visitor executed after translation, which verifies that all <see cref="SqlExpression" /> nodes have a type mapping,
 ///     and applies type mappings inferred for queryable constants (VALUES) and parameters (e.g. OPENJSON) back on their root tables.
 /// </summary>
-public class RelationalTypeMappingPostprocessor : ExpressionVisitor
+public class RelationalTypeMappingPostprocessor(
+    QueryTranslationPostprocessorDependencies dependencies,
+    RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
+    RelationalQueryCompilationContext queryCompilationContext)
+    : ExpressionVisitor
 {
-    private readonly ISqlExpressionFactory _sqlExpressionFactory;
+    private readonly ISqlExpressionFactory _sqlExpressionFactory = relationalDependencies.SqlExpressionFactory;
+
     private SelectExpression? _currentSelectExpression;
 
     /// <summary>
     ///     The inferred type mappings to be applied back on their query roots.
     /// </summary>
     private IReadOnlyDictionary<(string TableAlias, string ColumnName), RelationalTypeMapping?> _inferredTypeMappings = null!;
 
-    /// <summary>
-    ///     Creates a new instance of the <see cref="RelationalTypeMappingPostprocessor" /> class.
-    /// </summary>
-    /// <param name="dependencies">Parameter object containing dependencies for this class.</param>
-    /// <param name="relationalDependencies">Parameter object containing relational dependencies for this class.</param>
-    /// <param name="queryCompilationContext">The query compilation context object to use.</param>
-    public RelationalTypeMappingPostprocessor(
-        QueryTranslationPostprocessorDependencies dependencies,
-        RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
-        RelationalQueryCompilationContext queryCompilationContext)
-    {
-        Dependencies = dependencies;
-        RelationalDependencies = relationalDependencies;
-        QueryCompilationContext = queryCompilationContext;
-        _sqlExpressionFactory = relationalDependencies.SqlExpressionFactory;
-    }
-
     /// <summary>
     ///     Parameter object containing dependencies for this class.
     /// </summary>
-    protected virtual QueryTranslationPostprocessorDependencies Dependencies { get; }
+    protected virtual QueryTranslationPostprocessorDependencies Dependencies { get; } = dependencies;
 
     /// <summary>
     ///     Parameter object containing relational dependencies for this class.
     /// </summary>
-    protected virtual RelationalQueryTranslationPostprocessorDependencies RelationalDependencies { get; }
+    protected virtual RelationalQueryTranslationPostprocessorDependencies RelationalDependencies { get; } = relationalDependencies;
 
     /// <summary>
     ///     The query compilation context object to use.
     /// </summary>
-    protected virtual RelationalQueryCompilationContext QueryCompilationContext { get; }
+    protected virtual RelationalQueryCompilationContext QueryCompilationContext { get; } = queryCompilationContext;
 
     /// <summary>
     ///     Processes type mappings in the expression tree.
@@ -164,12 +152,13 @@ protected virtual ValuesExpression ApplyTypeMappingsOnValuesExpression(ValuesExp
 
         switch (valuesExpression)
         {
-            case { RowValues: not null }:
+            // Regular VALUES over a collection of scalar values. Apply the inferred type mappings on each of the values.
+            case { RowValues: IReadOnlyList<RowValueExpression> rowValues }:
             {
-                var newRowValues = new RowValueExpression[valuesExpression.RowValues.Count];
+                var newRowValues = new RowValueExpression[rowValues.Count];
                 for (var i = 0; i < newRowValues.Length; i++)
                 {
-                    var rowValue = valuesExpression.RowValues[i];
+                    var rowValue = rowValues[i];
                     var newValues = new SqlExpression[newColumnNames.Count];
                     for (var j = 0; j < valuesExpression.ColumnNames.Count; j++)
                     {
@@ -186,8 +175,8 @@ protected virtual ValuesExpression ApplyTypeMappingsOnValuesExpression(ValuesExp
                             value = _sqlExpressionFactory.ApplyTypeMapping(value, inferredTypeMapping);
                         }
 
-                        // We currently add explicit conversions on the first row (but not to the _ord column), to ensure that the inferred types
-                        // are properly typed. See #30605 for removing that when not needed.
+                        // We currently add explicit conversions on the first row (but not to the _ord column), to ensure that the inferred
+                        // types are properly typed. See #30605 for removing that when not needed.
                         if (i == 0 && j > 0 && value is not ColumnExpression)
                         {
                             value = new SqlUnaryExpression(ExpressionType.Convert, value, value.Type, value.TypeMapping);
@@ -198,29 +187,31 @@ protected virtual ValuesExpression ApplyTypeMappingsOnValuesExpression(ValuesExp
 
                     newRowValues[i] = new RowValueExpression(newValues);
                 }
+
                 return new ValuesExpression(valuesExpression.Alias, newRowValues, null, newColumnNames);
             }
 
-            case { ValuesParameter: not null }:
+            // VALUES over a values parameter (i.e. a parameter representing the entire collection, that will be constantized into the SQL
+            // later). Apply the inferred type mapping on the parameter.
+            case { ValuesParameter: { TypeMapping: null } valuesParameter }
+                when inferredTypeMappings[1] is RelationalTypeMapping elementTypeMapping:
             {
-                var valuesParameter = valuesExpression.ValuesParameter;
-                if (valuesParameter.TypeMapping is null
-                    && inferredTypeMappings[1] is RelationalTypeMapping elementTypeMapping)
+                if (RelationalDependencies.TypeMappingSource.FindMapping(
+                        valuesParameter.Type, QueryCompilationContext.Model, elementTypeMapping) is not
+                    { ElementTypeMapping: not null } collectionParameterTypeMapping)
                 {
-                    if (RelationalDependencies.TypeMappingSource.FindMapping(valuesParameter.Type, QueryCompilationContext.Model, elementTypeMapping) is not RelationalTypeMapping { ElementTypeMapping: not null } parameterTypeMapping)
-                    {
-                        throw new UnreachableException("A RelationalTypeMapping collection type mapping could not be found");
-                    }
-
-                    valuesParameter = (SqlParameterExpression)valuesParameter.ApplyTypeMapping(parameterTypeMapping);
+                    throw new UnreachableException("A RelationalTypeMapping collection type mapping could not be found");
                 }
 
-                return new ValuesExpression(valuesExpression.Alias, null, valuesParameter, newColumnNames);
+                return new ValuesExpression(
+                    valuesExpression.Alias,
+                    (SqlParameterExpression)valuesParameter.ApplyTypeMapping(collectionParameterTypeMapping),
+                    newColumnNames);
             }
 
             default:
                 throw new UnreachableException();
-        };
+        }
     }
 
     /// <summary>