fix: run a Compare over ranges in a query as the method the code called (#464)

CHANGELOG.md

@@ -27,7 +27,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11. Not
 - EF Core: `Contains(T)` on a range or multirange is `false` in a query when the element is NULL: a column of a missing child in an outer join, or a nullable column read with `.Value`. Its negation dropped the row, and a projection threw ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-predicates-over-a-null-range-or-a-null-column)).
 - EF Core: `CreateFinite`, `CreateUnboundedStart` and `CreateUnboundedEnd` over a NULL element are NULL in a query, as EF Core's own `AddDays` is. They built `empty` or `(,)`, so `Contains(v)` selected a row without the element ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-predicates-over-a-null-range-or-a-null-column)).
 - EF Core: a value set `Union` with a NULL set, or `Remove` with a NULL set or element, is NULL in a query, as `Where(… == null)` already said. A `Select` loaded the other set instead, for a nullable column, a missing outer-join child or a `null` parameter ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-value-set-union-and-remove-over-null-in-queries)).
-- EF Core: `RangeLowerBoundComparer.Compare`, and any other `IComparer` of ranges or sets, runs in memory only in a query. A filter or an ordering fails translation. In SQL it compared in the range order, so `Compare(r.Period, p) == 0` missed rows with the same lower bound ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#move-rangelowerboundcomparer-out-of-queries)).
+- EF Core: any `Compare(x, y)` over ranges or sets, `RangeLowerBoundComparer.Compare` and every other `IComparer` included, runs in memory only in a query, as the method called. A filter or an ordering fails translation. In SQL it compared in the range order, so `Compare(r.Period, p) == 0` missed rows with the same lower bound ([#464](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/464), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#move-rangelowerboundcomparer-out-of-queries)).
 
 ### Added
 

docs/efcore.md

@@ -938,7 +938,7 @@ translation, and in the final projection EF runs them in memory instead:
 | The value set indexer, `set[0]` | Subscripting reads the stored array, which is canonical only for rows this library wrote ([Rows written by other tools](#rows-written-by-other-tools)). PostgreSQL arrays also start at 1, and an index past the end is `NULL` where memory throws. |
 | `Values` on a value set | It is the loaded array itself. In a query, use the set's own members, such as `Contains` and `Count`. |
 | `Count` and the indexer on a `RangeSet` | Not translated. PostgreSQL has no function for either, and each would be a subquery over `unnest(x)`. |
-| `Compare` on `RangeLowerBoundComparer`, `RangeSet.LowerBoundComparer` and any other `IComparer` of ranges or sets | EF Core reads every `Compare(x, y)` as the natural order of `x` and `y`, whatever the comparer. In SQL the lower-bound comparer would compare in the range order. What counts is what you compare, not the type the comparer is declared over: an `IComparer<IRange<T>>`, a non-generic `IComparer` and a comparer of value sets run in memory too. For the range order in SQL, call `x.CompareTo(y)`. |
+| `Compare` on `RangeLowerBoundComparer`, `RangeSet.LowerBoundComparer`, any other `IComparer` of ranges or sets, and any other `Compare(x, y)` over them | EF Core reads every `Compare(x, y)` as the natural order of `x` and `y`, whatever the comparer. In SQL the lower-bound comparer would compare in the range order. What counts is what you compare, not the type the comparer is declared over: an `IComparer<IRange<T>>`, a non-generic `IComparer` and a comparer of value sets run in memory too. So do a `Compare` method on a class that isn't a comparer and a static `Compare`. The query runs the method you called, so a class whose public `Compare` differs from its `IComparer<T>.Compare` gets the public one, as in memory. For the range order in SQL, call `x.CompareTo(y)`. |
 | `From` and `Parse` on any range or set | They build a value in .NET. Given values your code passes in, EF Core runs them before the query and sends the result as a parameter. |
 | `ToInstantRange`, `ToInterval`, `ToLocalDateRange`, `ToDateInterval` and `ToYearMonthRange` (NodaTime package) | They convert in .NET: between NodaTime's `Interval` or `DateInterval` and a range type, or between `LocalDateRange` and `YearMonthRange`. |
 

docs/migration.md

@@ -996,10 +996,13 @@ in SQL in that order. Now it runs in memory only, like the other members under
 `Comparer<DateRange>.Default.Compare` runs in memory too. Once the comparer is a query parameter, a
 query can't tell one comparer from another. So do comparers declared over another type, when what they
 compare is a range or a set: an `IComparer<IRange<DateOnly>>`, a non-generic `IComparer` (such as
-`Comparer<DateRange>.Create(…)` held as one) and an `IComparer<StringSet>`.
+`Comparer<DateRange>.Create(…)` held as one) and an `IComparer<StringSet>`. The same goes for any
+other method named `Compare` that takes two ranges or two sets, such as a static helper or one on a
+class that isn't a comparer. The query runs the method you called, so a class whose public `Compare`
+differs from its `IComparer<T>.Compare` gets the public one, as in memory.
 
-- **Search your queries for `Compare(`.** Each call on a comparer of ranges or sets in a `Where` or
-  an `OrderBy` now throws, so running the query finds it.
+- **Search your queries for `Compare(`.** Each `Compare` over ranges or sets in a `Where` or an
+  `OrderBy` now throws, so running the query finds it.
 - **Filter and sort by the lower bound itself.** `LowerBound()` and `LowerBoundInclusive()` run as
   SQL, so `Where(r => r.Period.LowerBound() == day)` and `OrderBy(r => r.Period.LowerBound())` work.
   An ordering puts the rows without a lower bound where PostgreSQL puts NULL, last ascending

src/CodoMetis.ValueRanges.EFCore.PostgreSQL/CHANGELOG.md

@@ -16,7 +16,7 @@ The first preview of the 11 line: the same public API as 10.0.0, on .NET 11, EF
 - EF Core: `Contains(T)` on a range or multirange is `false` in a query when the element is NULL: a column of a missing child in an outer join, or a nullable column read with `.Value`. Its negation dropped the row, and a projection threw ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-predicates-over-a-null-range-or-a-null-column)).
 - EF Core: `CreateFinite`, `CreateUnboundedStart` and `CreateUnboundedEnd` over a NULL element are NULL in a query, as EF Core's own `AddDays` is. They built `empty` or `(,)`, so `Contains(v)` selected a row without the element ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-predicates-over-a-null-range-or-a-null-column)).
 - EF Core: a value set `Union` with a NULL set, or `Remove` with a NULL set or element, is NULL in a query, as `Where(… == null)` already said. A `Select` loaded the other set instead, for a nullable column, a missing outer-join child or a `null` parameter ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#check-value-set-union-and-remove-over-null-in-queries)).
-- EF Core: `RangeLowerBoundComparer.Compare`, and any other `IComparer` of ranges or sets, runs in memory only in a query. A filter or an ordering fails translation. In SQL it compared in the range order, so `Compare(r.Period, p) == 0` missed rows with the same lower bound ([Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#move-rangelowerboundcomparer-out-of-queries)).
+- EF Core: any `Compare(x, y)` over ranges or sets, `RangeLowerBoundComparer.Compare` and every other `IComparer` included, runs in memory only in a query, as the method called. A filter or an ordering fails translation. In SQL it compared in the range order, so `Compare(r.Period, p) == 0` missed rows with the same lower bound ([#464](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/issues/464), [Migration](https://github.com/CaffeinatedCoder/CodoMetis.ValueRanges/blob/main/docs/migration.md#move-rangelowerboundcomparer-out-of-queries)).
 
 ### Added
 

src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/RangeComparerCall.cs

@@ -1,31 +1,72 @@
+using System.Collections.Concurrent;
+using System.Linq.Expressions;
 using System.Reflection;
 
 namespace CodoMetis.ValueRanges.EntityFrameworkCore.PostgreSQL.Query;
 
 /// <summary>
-/// The form an <see cref="IComparer{T}"/>'s or <see cref="System.Collections.IComparer"/>'s <c>Compare</c>
-/// over a range, a range set or a value set takes in a query
-/// once <see cref="ValueRangesQueryExpressionInterceptor"/> has seen it: a call no translator knows, so
-/// it runs in memory or not at all (#385).
+/// The form a <c>Compare(x, y)</c> over a range, a range set or a value set takes in a query once
+/// <see cref="ValueRangesQueryExpressionInterceptor"/> has seen it: a call no translator knows, so it
+/// runs in memory or not at all (#385).
 /// </summary>
 /// <remarks>
 /// EF Core reads any <c>Compare(x, y)</c> as the natural order of its operands, so
 /// <see cref="RangeLowerBoundComparer{TRange,T}"/> ran in SQL in the range order instead of by lower
 /// bound. Under this name EF Core fails to translate the call in a filter or an ordering, and its message
-/// shows <c>RangeComparerCall.RunInMemory(…)</c>; in a final projection it calls the comparer on the rows
-/// it loaded, which is the answer the same code gives in memory.
+/// shows <c>RangeComparerCall.RunInMemory(…)</c>; in a final projection it runs the call on the rows it
+/// loaded, which is the answer the same code gives in memory. The call runs as written: the method the
+/// code called, not <see cref="IComparer{T}.Compare"/>, which a class may implement differently, and
+/// which a <c>Compare</c> on a class that is no comparer, or a static one, does not have (#464).
 /// </remarks>
 internal static class RangeComparerCall
 {
-    /// <summary><paramref name="comparer"/>'s own <see cref="IComparer{T}.Compare"/>.</summary>
-    public static int RunInMemory<T>(IComparer<T> comparer, T x, T y) => comparer.Compare(x, y);
+    /// <summary>The call <paramref name="call"/> stands for, on <paramref name="comparer"/>.</summary>
+    public static int RunInMemory<TComparer, T>(TComparer comparer, T x, T y, int call) => ((Func<TComparer, T, T, int>)Calls[call])(comparer, x, y);
 
-    /// <summary><paramref name="comparer"/>'s own <see cref="System.Collections.IComparer.Compare"/>.</summary>
-    public static int RunInMemory(System.Collections.IComparer comparer, object? x, object? y) => comparer.Compare(x, y);
+    /// <summary>The static call <paramref name="call"/> stands for.</summary>
+    public static int RunInMemory<T>(T x, T y, int call) => ((Func<T, T, int>)Calls[call])(x, y);
 
-    internal static readonly MethodInfo RunInMemoryMethod =
-        typeof(RangeComparerCall).GetMethods().Single(method => method is { Name: nameof(RunInMemory), IsGenericMethodDefinition: true });
+    private static readonly MethodInfo InstanceMethod =
+        typeof(RangeComparerCall).GetMethods().Single(method => method is { Name: nameof(RunInMemory) } && method.GetGenericArguments().Length == 2);
 
-    internal static readonly MethodInfo RunInMemoryNonGenericMethod =
-        typeof(RangeComparerCall).GetMethod(nameof(RunInMemory), [typeof(System.Collections.IComparer), typeof(object), typeof(object)])!;
+    private static readonly MethodInfo StaticMethod =
+        typeof(RangeComparerCall).GetMethods().Single(method => method is { Name: nameof(RunInMemory) } && method.GetGenericArguments().Length == 1);
+
+    // The query holds the call as a number: EF Core refuses a delegate constant in a client projection,
+    // since it would be captured by the cached query, and accepts a constant of a type it maps. One
+    // delegate per method and receiver type; a race may compile one twice, and the extra is never used.
+    private static readonly ConcurrentDictionary<(MethodInfo Method, Type? Receiver), int> Ids = new();
+
+    private static readonly ConcurrentDictionary<int, Delegate> Calls = new();
+
+    private static int _lastId;
+
+    /// <summary>
+    /// <paramref name="method"/> called on <paramref name="comparer"/> (<see langword="null"/> for a
+    /// static method) with <paramref name="x"/> and <paramref name="y"/>, as a <c>RunInMemory</c> call.
+    /// </summary>
+    public static MethodCallExpression Of(MethodInfo method, Expression? comparer, Expression x, Expression y)
+    {
+        var compared = method.GetParameters()[0].ParameterType;
+        var call = Expression.Constant(Ids.GetOrAdd((method, comparer?.Type), static key =>
+        {
+            var id = Interlocked.Increment(ref _lastId);
+            Calls[id] = Compile(key.Method, key.Receiver);
+            return id;
+        }));
+
+        return comparer is null
+            ? Expression.Call(StaticMethod.MakeGenericMethod(compared), x, y, call)
+            : Expression.Call(InstanceMethod.MakeGenericMethod(comparer.Type, compared), comparer, x, y, call);
+    }
+
+    private static Delegate Compile(MethodInfo method, Type? receiverType)
+    {
+        var (x, y) = (Expression.Parameter(method.GetParameters()[0].ParameterType), Expression.Parameter(method.GetParameters()[1].ParameterType));
+        if (receiverType is null)
+            return Expression.Lambda(Expression.Call(method, x, y), x, y).Compile();
+
+        var receiver = Expression.Parameter(receiverType);
+        return Expression.Lambda(Expression.Call(receiver, method, x, y), receiver, x, y).Compile();
+    }
 }

src/CodoMetis.ValueRanges.EFCore.PostgreSQL/Query/ValueRangesQueryExpressionInterceptor.cs

@@ -27,9 +27,9 @@ namespace CodoMetis.ValueRanges.EntityFrameworkCore.PostgreSQL.Query;
 /// read-only collection — see <see cref="RangeSetContainsRewriter"/>.
 /// </para>
 /// <para>
-/// A sixth keeps an <see cref="IComparer{T}"/> over ranges or sets,
-/// <see cref="RangeLowerBoundComparer{TRange,T}"/> among them, out of SQL, where EF Core would read it as the
-/// natural order — see <see cref="RangeComparerRewriter"/>.
+/// A sixth keeps a <c>Compare(x, y)</c> over ranges or sets, an <see cref="IComparer{T}"/>'s and
+/// <see cref="RangeLowerBoundComparer{TRange,T}"/>'s among them, out of SQL, where EF Core would read it as
+/// the natural order — see <see cref="RangeComparerRewriter"/>.
 /// </para>
 /// <para>
 /// The value set refusals — a server-computed <c>Union</c> consumed by an order- or
@@ -363,9 +363,10 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
     }
 
     /// <summary>
-    /// Turns <c>comparer.Compare(x, y)</c> over a range, a range set or a value set into
-    /// <see cref="RangeComparerCall.RunInMemory{T}"/>, which EF Core cannot translate: a filter or an
-    /// ordering then fails translation, and a final projection runs the comparer in memory (#385).
+    /// Turns <c>comparer.Compare(x, y)</c> or a static <c>Compare(x, y)</c> over a range, a range set or a
+    /// value set into <see cref="RangeComparerCall.RunInMemory{TComparer,T}"/>, which EF Core cannot
+    /// translate: a filter or an ordering then fails translation, and a final projection runs the call in
+    /// memory (#385).
     /// </summary>
     /// <remarks>
     /// <para>
@@ -391,28 +392,29 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
     /// (#452). So a call is rewritten when either operand, under the conversions to the declared type, is a
     /// range, a range set or a value set.
     /// </para>
+    /// <para>
+    /// Nor does the declaring type decide whether a call is rewritten, or which method runs. EF Core
+    /// translates every <c>int Compare(T, T)</c>, static or not, a comparer's or not, and a class may
+    /// implement <see cref="IComparer{T}.Compare"/> explicitly and differently from its public
+    /// <c>Compare</c>. Calling <see cref="IComparer{T}.Compare"/> in place of the method the code called
+    /// answered the explicit implementation's 0 where memory answered 1, and a <c>Compare</c> on a class
+    /// that is no comparer, or a static one, was not rewritten and ran in SQL in the range order (#464). So
+    /// <see cref="RangeComparerCall.Of"/> runs the call as written.
+    /// </para>
     /// </remarks>
     private sealed class RangeComparerRewriter : ExpressionVisitor
     {
         public static readonly RangeComparerRewriter Instance = new();
 
         protected override Expression VisitMethodCall(MethodCallExpression node)
-            => node is { Object: { } comparer, Method: { Name: nameof(IComparer<>.Compare) } method, Arguments: [var x, var y] }
+            => node is { Method: { Name: nameof(IComparer<>.Compare) } method, Arguments: [var x, var y] }
                && method.ReturnType == typeof(int)
                && method.GetParameters() is [{ ParameterType: var compared }, { ParameterType: var other }]
                && compared == other
                && (IsRangeOrSet(x) || IsRangeOrSet(y))
-               && RunInMemory(comparer.Type, compared) is { } runInMemory
-                   ? Expression.Call(runInMemory, Visit(comparer), Visit(x), Visit(y))
+                   ? RangeComparerCall.Of(method, node.Object is { } comparer ? Visit(comparer) : null, Visit(x), Visit(y))
                    : base.VisitMethodCall(node);
 
-        private static MethodInfo? RunInMemory(Type comparerType, Type compared)
-            => typeof(IComparer<>).MakeGenericType(compared).IsAssignableFrom(comparerType)
-                   ? RangeComparerCall.RunInMemoryMethod.MakeGenericMethod(compared)
-                   : compared == typeof(object) && typeof(System.Collections.IComparer).IsAssignableFrom(comparerType)
-                       ? RangeComparerCall.RunInMemoryNonGenericMethod
-                       : null;
-
         // The operand and every conversion it passes through on its way to the declared parameter type:
         // a column converted to IRange<T>, object or IComparable is still a range.
         private static bool IsRangeOrSet(Expression operand)