EF Core: Min, Max, Order or OrderBy(x => x) over a value object boxed to object orders by the column, not its CompareTo #206

Found by a sweep of release/2.3.1 @ dbdd44e (lens: framework-heuristics, run-time satellites) for the defect class of #191-#197; reproduced in a scratch test on SQLite.

Package

CodoMetis.TypeKit.EntityFrameworkCore

Version

2.3.0 (the same code in v2.3.0; the key-selector refusal arrived in 8f6af8b), and release/2.3.1 @ dbdd44e

How does it fail?

Silently. The query runs and answers the column's order, the opposite of memory's

Problem

HandWrittenOrderingInterceptor.Finder.VisitMethodCall (src/CodoMetis.TypeKit.EntityFrameworkCore/HandWrittenOrderingInterceptor.cs:113-120) refuses by OrderedType(method), the operator's declared type, and unwraps a boxing conversion (Unconverted) only in a key selector's body. When the value object is boxed before the operator, by a Select(r => (object)r.Rank), the declared type is object, so Min, Max, Order, OrderDescending and OrderBy(x => x) are not refused. EF drops the cast and orders by the column.

Scenario

ProbeDescending (hand-written descending CompareTo) holding 1, 2, 3, measured @ dbdd44e on SQLite:

db.Ranked.Select(r => (object)r.Rank).Max();               // 3; memory: 1
db.Ranked.Select(r => (object)r.Rank).Min();               // 1; memory: 3
db.Ranked.Select(r => (object)r.Rank).Order();             // 1,2,3; memory: 3,2,1
db.Ranked.Select(r => (object)r.Rank).OrderBy(x => x);     // 1,2,3; memory: 3,2,1
db.Ranked.Select(r => (object)r.Label).Max();              // "3"; memory: "1" (record class ProbeReversedLabel)

Each, as a row of HandWrittenOrderingTests.Refused, fails with "should throw System.InvalidOperationException but did not". The existing rows OrderBy a key cast to object and ThenBy a key cast to object are refused.

A query filter goes through the same Finder (HandWrittenOrderingQueryFilterConvention): traced, not run. A cast to IComparable or IComparable<T> instead of object fails translation on SQLite today, so it is loud, but takes the same path.

Change

Judge an ordering operator by the type under its boxing conversions: for an operator without a selector, follow its source back through a Select whose selector body is a conversion with Method: null, and for a selector whose body is its own parameter, do the same for the source's element. In short, the value object under the conversion, not the declared object.

Test

The five rows above in HandWrittenOrderingTests.Refused, one boxed row in the query-filter theory, and OrderDescending; reverted per verify-the-guard. Input class (GTS): an ordering operator over a value object, by where the conversion sits (key selector, source projection, identity selector), against QueryShapes.

Environment

.NET SDK 10.0.4xx, EF Core 10, SQLite; any provider.

Fixed in #217 (3d2abfd)

The Finder reads the element types under the boxing, through a Select and the operators that pass elements on (Where, Take, Distinct, orderings, Cast/OfType, Concat and its kind), for an operator without a key or whose key is the element itself. The query filter case is refused at model building (run, not only traced). Sibling found: over a navigation the same query threw a NullReferenceException inside EF; it is refused before EF now. QueryShapes gains four boxed operations over every kind.

Two more shapes, fixed in #217 by d573eef

Reported by an external review @ dbdd44e, which measured both as 1,2,3 where memory answers 3,2,1:

db.Ranked.OrderBy(r => (object?)r.Rank ?? fallback);                          // a coalesced boxed key
db.Ranked.Select(r => new { Key = (object)r.Rank }).OrderBy(a => a.Key);      // a boxed key behind an anonymous projection

Unconverted strips only a conversion, so a key whose body is a ?? over a boxing stays object; and ElementTypes follows a Select whose body is a conversion, not a member of a projected anonymous (or other) type that holds one. Both belong in this fix: the coalesce's operands unconverted, and a key that reads a member of a projection followed back to that member's initializer. QueryShapes gains both, by where the boxing sits.

d573eef reads a key or selector body through both sides of a ?? and a ?:, and follows a member of a projection (anonymous new or member initializer, nested too) back to the expression that set it. Siblings fixed with it: Max(a => a.Key) and Select(a => a.Key).Max() over the projection, a Where in between, a ?? inside the projection, and over a navigation (a NullReferenceException inside EF before). QueryShapes gains OrderByCoalescedBoxed, OrderByProjectedBoxed and MaxProjectedBoxed; reverting the source fails 29 rows.

Eight more shapes, fixed in #217 by 8562de6

Measured on SQLite at d573eef (ProbeDescending 1, 2, 3; memory's ascending order is 3, 2, 1 and its Max is 1). d573eef follows a projection's member back through Select only, and an element back through the operators that pass elements on; these reach the boxed value through other operators:

db.Ranked.GroupBy(r => (object)r.Rank).OrderBy(g => g.Key);                                    // 1,2,3: a grouping's boxed key
db.Ranked.GroupBy(r => r.Id % 1, r => (object)r.Rank).Select(g => g.Max());                   // 3: a grouping's boxed elements
db.Ranked.GroupBy(r => r.Rank, (k, g) => new { Key = (object)k }).OrderBy(a => a.Key);        // 1,2,3: GroupBy's result selector
db.Ranked.Join(db.Ranked, a => a.Id, b => b.Id, (a, b) => new { Key = (object)a.Rank }).OrderBy(x => x.Key);  // 1,2,3
db.Parents.SelectMany(p => p.Children, (p, c) => new { Key = (object)c.Rank }).OrderBy(x => x.Key);           // 1,2,3
db.Parents.SelectMany(p => p.Children.Select(c => (object)c.Rank)).Max();                     // 3
db.Ranked.OrderBy(r => db.Ranked.Where(x => x.Id == r.Id).Select(x => (object)x.Rank).First());               // 1,2,3: a subquery's boxed First
db.Ranked.OrderBy(r => db.Parents.SelectMany(p => p.Children).Where(c => c.Id == r.Id).Select(c => (object)c.Rank).FirstOrDefault()); // 1,2,3

Loud today, same path: Select(r => Tuple.Create((object)r.Rank, r.Rank)).OrderBy(t => t.Item1) fails translation on SQLite. Already refused: GroupJoin whose result reads cs.Max(c => c.Rank), by the inner Max.

Change: follow a lambda's parameters to what they range over for every operator that binds one: GroupBy's key, element and result selectors (g.Key to the key selector, g's elements to the element selector), Join's and GroupJoin's result selector (each parameter to its sequence), SelectMany's collection and result selectors; and read the element of First, FirstOrDefault, Single, SingleOrDefault, Last, LastOrDefault, ElementAt and ElementAtOrDefault through the sequence they take it from. A row each in HandWrittenOrderingTests.Refused, and QueryShapes ops for the grouping key and a Join result.

8562de6 binds every LINQ operator's lambda parameters to what they stand for (the source's elements; each side of a Join/GroupJoin; SelectMany's collection; GroupBy's key and elements), reads a grouping's Key from its key selector and First and its kind from their source, and carries the bindings into nested lambdas, so g.Max() is refused too. Sibling: a GroupJoin result reading cs.First().Rank. QueryShapes gains OrderByGroupKeyBoxed and OrderByJoinedBoxed; reverting the source fails 21 rows. Tuple.Create stays as it was: EF does not translate it.