SqlServerAggregateFunctionExpression no longer extends SqlFunctionExpression

Author: roji

src/EFCore.Relational/Query/SqlExpressions/SqlFunctionExpression.cs

@@ -250,7 +250,6 @@ private SqlFunctionExpression(
     /// <summary>
     ///     A list of bool values indicating whether individual argument propagate null to the result.
     /// </summary>
-
     public virtual IReadOnlyList<bool>? ArgumentsPropagateNullability { get; }
 
     /// <inheritdoc />

src/EFCore.SqlServer/Query/Internal/SqlServerAggregateFunctionExpression.cs

@@ -11,7 +11,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
 ///     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 class SqlServerAggregateFunctionExpression : SqlFunctionExpression, IEquatable<SqlServerAggregateFunctionExpression>
+public class SqlServerAggregateFunctionExpression : SqlExpression, IEquatable<SqlServerAggregateFunctionExpression>
 {
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -20,15 +20,31 @@ public class SqlServerAggregateFunctionExpression : SqlFunctionExpression, IEqua
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public SqlServerAggregateFunctionExpression(
-        string functionName,
-        IEnumerable<SqlExpression> arguments,
+        string name,
+        IReadOnlyList<SqlExpression> arguments,
         IReadOnlyList<OrderingExpression> orderings,
         bool nullable,
         IEnumerable<bool> argumentsPropagateNullability,
         Type type,
         RelationalTypeMapping? typeMapping)
-        : base(functionName, arguments, nullable, argumentsPropagateNullability, type, typeMapping)
-        => Orderings = orderings;
+        : base(type, typeMapping)
+    {
+        Name = name;
+        Arguments = arguments.ToList();
+        Orderings = orderings;
+        IsNullable = nullable;
+        ArgumentsPropagateNullability = argumentsPropagateNullability.ToList();
+    }
+
+    /// <summary>
+    ///     The name of the function.
+    /// </summary>
+    public virtual string Name { get; }
+
+    /// <summary>
+    ///     The list of arguments of this function.
+    /// </summary>
+    public virtual IReadOnlyList<SqlExpression> Arguments { get; }
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -38,6 +54,16 @@ public SqlServerAggregateFunctionExpression(
     /// </summary>
     public virtual IReadOnlyList<OrderingExpression> Orderings { get; }
 
+    /// <summary>
+    ///     A bool value indicating if the function can return null result.
+    /// </summary>
+    public virtual bool IsNullable { get; }
+
+    /// <summary>
+    ///     A list of bool values indicating whether individual argument propagate null to the result.
+    /// </summary>
+    public virtual IReadOnlyList<bool> ArgumentsPropagateNullability { get; }
+
     /// <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
@@ -46,54 +72,70 @@ public SqlServerAggregateFunctionExpression(
     /// </summary>
     protected override Expression VisitChildren(ExpressionVisitor visitor)
     {
-        var visitedBase = (SqlFunctionExpression)base.VisitChildren(visitor);
+        SqlExpression[]? arguments = null;
+        for (var i = 0; i < Arguments.Count; i++)
+        {
+            var visitedArgument = (SqlExpression)visitor.Visit(Arguments[i]);
+            if (visitedArgument != Arguments[i] && arguments is null)
+            {
+                arguments = new SqlExpression[Arguments.Count];
+
+                for (var j = 0; j < i; j++)
+                {
+                    arguments[j] = Arguments[j];
+                }
+            }
 
-        OrderingExpression[]? visitedAggregateOrderings = null;
+            if (arguments is not null)
+            {
+                arguments[i] = visitedArgument;
+            }
+        }
 
+        OrderingExpression[]? orderings = null;
         for (var i = 0; i < Orderings.Count; i++)
         {
             var visitedOrdering = (OrderingExpression)visitor.Visit(Orderings[i]);
-            if (visitedOrdering != Orderings[i] && visitedAggregateOrderings is null)
+            if (visitedOrdering != Orderings[i] && orderings is null)
             {
-                visitedAggregateOrderings = new OrderingExpression[Orderings.Count];
+                orderings = new OrderingExpression[Orderings.Count];
 
-                for (var j = 0; j < visitedAggregateOrderings.Length; j++)
+                for (var j = 0; j < i; j++)
                 {
-                    visitedAggregateOrderings[j] = Orderings[j];
+                    orderings[j] = Orderings[j];
                 }
             }
 
-            if (visitedAggregateOrderings is not null)
+            if (orderings is not null)
             {
-                visitedAggregateOrderings[i] = visitedOrdering;
+                orderings[i] = visitedOrdering;
             }
         }
 
-        return visitedBase != this || visitedAggregateOrderings is not null
+        return arguments is not null || orderings is not null
             ? new SqlServerAggregateFunctionExpression(
                 Name,
-                visitedBase.Arguments!,
-                visitedAggregateOrderings ?? Orderings,
+                arguments ?? Arguments,
+                orderings ?? Orderings,
                 IsNullable,
-                ArgumentsPropagateNullability!,
+                ArgumentsPropagateNullability,
                 Type,
                 TypeMapping)
             : this;
     }
 
     /// <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.
+    ///     Applies supplied type mapping to this expression.
     /// </summary>
-    public override SqlServerAggregateFunctionExpression ApplyTypeMapping(RelationalTypeMapping? typeMapping)
+    /// <param name="typeMapping">A relational type mapping to apply.</param>
+    /// <returns>A new expression which has supplied type mapping.</returns>
+    public virtual SqlServerAggregateFunctionExpression ApplyTypeMapping(RelationalTypeMapping? typeMapping)
         => new(
             Name,
-            Arguments!,
+            Arguments,
             Orderings,
             IsNullable,
-            ArgumentsPropagateNullability!,
+            ArgumentsPropagateNullability,
             Type,
             typeMapping ?? TypeMapping);
 
@@ -103,33 +145,29 @@ public override SqlServerAggregateFunctionExpression ApplyTypeMapping(Relational
     ///     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 override SqlFunctionExpression Update(SqlExpression? instance, IReadOnlyList<SqlExpression>? arguments)
-    {
-        Check.DebugAssert(arguments is not null, "arguments is not null");
-        Check.DebugAssert(instance is null, "instance not supported on SqlServerFunctionExpression");
-
-        return arguments.SequenceEqual(Arguments!)
-            ? this
-            : new SqlServerAggregateFunctionExpression(
-                Name, arguments, Orderings, IsNullable, ArgumentsPropagateNullability!, Type, TypeMapping);
-    }
-
-    /// <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 SqlFunctionExpression UpdateOrderings(IReadOnlyList<OrderingExpression> orderings)
-        => orderings == Orderings || orderings.SequenceEqual(Orderings)
+    public virtual SqlServerAggregateFunctionExpression Update(
+        IReadOnlyList<SqlExpression> arguments,
+        IReadOnlyList<OrderingExpression> orderings)
+        => (ReferenceEquals(arguments, Arguments) || arguments.SequenceEqual(Arguments))
+            && (ReferenceEquals(orderings, Orderings) || orderings.SequenceEqual(Orderings))
             ? this
             : new SqlServerAggregateFunctionExpression(
-                Name, Arguments!, orderings, IsNullable, ArgumentsPropagateNullability!, Type, TypeMapping);
+                Name,
+                arguments,
+                orderings,
+                IsNullable,
+                ArgumentsPropagateNullability,
+                Type,
+                TypeMapping);
 
     /// <inheritdoc />
     protected override void Print(ExpressionPrinter expressionPrinter)
     {
-        base.Print(expressionPrinter);
+        expressionPrinter.Append(Name);
+
+        expressionPrinter.Append("(");
+        expressionPrinter.VisitCollection(Arguments);
+        expressionPrinter.Append(")");
 
         if (Orderings.Count > 0)
         {
@@ -146,18 +184,27 @@ public override bool Equals(object? obj)
     /// <inheritdoc />
     public virtual bool Equals(SqlServerAggregateFunctionExpression? other)
         => ReferenceEquals(this, other)
-            || base.Equals(other) && Orderings.SequenceEqual(other.Orderings);
+            || other is not null
+            && base.Equals(other)
+            && Name == other.Name
+            && Arguments.SequenceEqual(other.Arguments)
+            && Orderings.SequenceEqual(other.Orderings);
 
     /// <inheritdoc />
     public override int GetHashCode()
     {
         var hash = new HashCode();
-
         hash.Add(base.GetHashCode());
+        hash.Add(Name);
+
+        for (var i = 0; i < Arguments.Count; i++)
+        {
+            hash.Add(Arguments[i]);
+        }
 
-        foreach (var orderingExpression in Orderings)
+        for (var i = 0; i < Orderings.Count; i++)
         {
-            hash.Add(orderingExpression);
+            hash.Add(Orderings[i]);
         }
 
         return hash.ToHashCode();

src/EFCore.SqlServer/Query/Internal/SqlServerExpression.cs

@@ -43,7 +43,7 @@ public static SqlFunctionExpression AggregateFunction(
     ///     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 static SqlFunctionExpression AggregateFunctionWithOrdering(
+    public static SqlExpression AggregateFunctionWithOrdering(
         ISqlExpressionFactory sqlExpressionFactory,
         string name,
         IEnumerable<SqlExpression> arguments,