diff --git a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFrameBoundSqlExpression.cs b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFrameBoundSqlExpression.cs index 9ffc9f27..ad1d714f 100644 --- a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFrameBoundSqlExpression.cs +++ b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFrameBoundSqlExpression.cs @@ -30,7 +30,7 @@ public override Expression Quote() => #endif public override bool Equals(object? obj) => - obj is WindowFrameBoundSqlExpression other && BoundInfo == other.BoundInfo; + obj is WindowFrameBoundSqlExpression other && base.Equals(other) && BoundInfo == other.BoundInfo; - public override int GetHashCode() => BoundInfo.GetHashCode(); + public override int GetHashCode() => HashCode.Combine(base.GetHashCode(), BoundInfo); } diff --git a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpression.cs b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpression.cs index e1c13007..a939b37d 100644 --- a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpression.cs +++ b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpression.cs @@ -10,9 +10,11 @@ namespace ExpressiveSharp.EntityFrameworkCore.RelationalExtensions.Infrastructur /// RANK, DENSE_RANK, NTILE, aggregates, and navigation functions (ROW_NUMBER uses the built-in /// ). /// -/// Self-rendering: interleaves -/// nodes with column/ordering expressions, producing correct SQL through any provider's -/// — no custom generator replacement needed. +/// Self-rendering: for visitors, +/// interleaves nodes with column/ordering expressions, +/// producing correct SQL through any provider's generator — no custom generator replacement +/// needed. All other visitors rebuild children normally so rewrites (alias remapping, pushdown) +/// are preserved. /// /// /// Function names and clause syntax are hardcoded as literal SQL fragments, relying on SQL:2003 @@ -53,10 +55,44 @@ public WindowFunctionSqlExpression( protected override Expression VisitChildren(ExpressionVisitor visitor) { - EmitWindowFunction( - text => visitor.Visit(new SqlFragmentExpression(text)), - expr => visitor.Visit(expr)); - return this; + if (visitor is QuerySqlGenerator) + { + EmitWindowFunction( + text => visitor.Visit(new SqlFragmentExpression(text)), + expr => visitor.Visit(expr)); + return this; + } + + // Every other visitor (projection pushdown, alias uniquification, ...) rewrites children; + // discarding its results would leave stale references in the generated SQL. + var changed = false; + + var newArguments = new SqlExpression[Arguments.Count]; + for (var i = 0; i < Arguments.Count; i++) + { + newArguments[i] = (SqlExpression)visitor.Visit(Arguments[i]); + changed |= newArguments[i] != Arguments[i]; + } + + var newPartitions = new SqlExpression[Partitions.Count]; + for (var i = 0; i < Partitions.Count; i++) + { + newPartitions[i] = (SqlExpression)visitor.Visit(Partitions[i]); + changed |= newPartitions[i] != Partitions[i]; + } + + var newOrderings = new OrderingExpression[Orderings.Count]; + for (var i = 0; i < Orderings.Count; i++) + { + newOrderings[i] = (OrderingExpression)visitor.Visit(Orderings[i]); + changed |= newOrderings[i] != Orderings[i]; + } + + return changed + ? new WindowFunctionSqlExpression( + FunctionName, newArguments, newPartitions, newOrderings, Type, TypeMapping, + FrameType, FrameStart, FrameEnd) + : this; } protected override void Print(ExpressionPrinter expressionPrinter) => @@ -119,6 +155,7 @@ public override Expression Quote() => public override bool Equals(object? obj) => obj is WindowFunctionSqlExpression other + && base.Equals(other) && FunctionName == other.FunctionName && Arguments.SequenceEqual(other.Arguments) && Partitions.SequenceEqual(other.Partitions) @@ -130,6 +167,7 @@ obj is WindowFunctionSqlExpression other public override int GetHashCode() { var hash = new HashCode(); + hash.Add(base.GetHashCode()); hash.Add(FunctionName); foreach (var a in Arguments) hash.Add(a); foreach (var p in Partitions) hash.Add(p); diff --git a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpressionWrapper.cs b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpressionWrapper.cs index 5caf39e6..9e746c12 100644 --- a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpressionWrapper.cs +++ b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowFunctionSqlExpressionWrapper.cs @@ -54,7 +54,7 @@ private sealed class UnwrapVisitor : ExpressionVisitor { if (node is SqlFragmentExpression fragment && fragment.Sql.StartsWith(PlaceholderPrefix, StringComparison.Ordinal) - && _stash.Remove(fragment.Sql, out var original)) + && _stash.TryGetValue(fragment.Sql, out var original)) { return original; } diff --git a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowSpecSqlExpression.cs b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowSpecSqlExpression.cs index 049e1da3..6977561c 100644 --- a/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowSpecSqlExpression.cs +++ b/src/ExpressiveSharp.EntityFrameworkCore.RelationalExtensions/Infrastructure/Internal/WindowSpecSqlExpression.cs @@ -119,6 +119,7 @@ public override Expression Quote() => public override bool Equals(object? obj) => obj is WindowSpecSqlExpression other + && base.Equals(other) && Partitions.SequenceEqual(other.Partitions) && Orderings.SequenceEqual(other.Orderings) && FrameType == other.FrameType @@ -128,6 +129,7 @@ obj is WindowSpecSqlExpression other public override int GetHashCode() { var hash = new HashCode(); + hash.Add(base.GetHashCode()); foreach (var p in Partitions) hash.Add(p); foreach (var o in Orderings) hash.Add(o); hash.Add(FrameType); diff --git a/tests/ExpressiveSharp.Tests/RelationalExtensions/WindowFunctionSqlExpressionTests.cs b/tests/ExpressiveSharp.Tests/RelationalExtensions/WindowFunctionSqlExpressionTests.cs new file mode 100644 index 00000000..b6c8c77d --- /dev/null +++ b/tests/ExpressiveSharp.Tests/RelationalExtensions/WindowFunctionSqlExpressionTests.cs @@ -0,0 +1,75 @@ +#pragma warning disable EF1001 + +using System.Linq.Expressions; +using ExpressiveSharp.EntityFrameworkCore.RelationalExtensions.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.Query; +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace ExpressiveSharp.Tests.RelationalExtensions; + +[TestClass] +public class WindowFunctionSqlExpressionTests +{ + private static readonly SqlFragmentExpression ColPrice = new("[Price]"); + private static readonly SqlFragmentExpression ColTotal = new("[Total]"); + private static readonly SqlFragmentExpression ColCustomerId = new("[CustomerId]"); + + private static string Print(Expression expr) + { + var printer = new ExpressionPrinter(); + printer.Visit(expr); + return printer.ToString(); + } + + private static WindowFunctionSqlExpression MakeSum(Type type) => + new( + "SUM", + arguments: [ColPrice], + partitions: [ColCustomerId], + orderings: [new OrderingExpression(ColPrice, ascending: true)], + type: type, + typeMapping: null); + + private sealed class ReplacingVisitor(Expression from, Expression to) : ExpressionVisitor + { + public override Expression? Visit(Expression? node) + => ReferenceEquals(node, from) ? to : base.Visit(node); + } + + [TestMethod] + public void VisitChildren_ReplacedArgument_IsReflectedInTheResult() + { + var expr = MakeSum(typeof(double)); + + var visited = new ReplacingVisitor(ColPrice, ColTotal).Visit(expr)!; + + var printed = Print(visited); + StringAssert.Contains(printed, "[Total]"); + } + + [TestMethod] + public void Equals_DifferingClrType_IsNotEqual() + { + var asDouble = MakeSum(typeof(double)); + var asDecimal = MakeSum(typeof(decimal)); + + Assert.AreNotEqual(asDouble, asDecimal); + } + + [TestMethod] + public void UnwrapAll_DuplicatedPlaceholder_RestoresBothOccurrences() + { + var windowFunction = MakeSum(typeof(double)); + + var wrapped = WindowFunctionSqlExpressionWrapper.WrapAll(windowFunction, out var stash); + Assert.AreEqual(1, stash.Count); + + var duplicated = Expression.Block(wrapped, wrapped); + + var unwrapped = (BlockExpression)WindowFunctionSqlExpressionWrapper.UnwrapAll(duplicated, stash); + + Assert.IsInstanceOfType(unwrapped.Expressions[0]); + Assert.IsInstanceOfType(unwrapped.Expressions[1]); + } +}