fix(query): return the real result of terminals after Select and after an indexed top-N window (#181)

[CaffeinatedCoder]

Fixes #181.

Terminals after a Select returned false or 0. While testing this I found two more paths that dropped or misread the operators after the part of the query they handle. All three now return what LINQ-to-Objects returns.

Changes

1. TryPushDownSelect no longer uses default as "not applicable" (the bug reported in #181)

• It returns bool with the projection in an out parameter. For a value-type result, default(TResult) is false or 0, and the provider returned that as the answer: Select(p => p.Age).Any(), .Max(), .Sum(), .Count(pred), .First(pred), Where(...).Select(...).Any(), and so on.
• The push-down is now tried only when the projection is the whole query (!HasComplexOperators).

2. A scalar terminal after Select on the direct pipeline

• Take(n).Select(...).Count(), Skip(n).Select(...).LongCount() and OrderBy(...).Select(...).Take(n).Count() reached ProjectEnumerable, which returned default for any non-sequence result. They now run through EnumerableRewriter, like the other shapes the pipeline doesn't model.
• ProjectEnumerable throws if it is ever reached with a non-sequence result again, instead of returning default.

3. The OrderBy(indexed).Skip/Take fast path

• The shortcut that reads only the window from the index used to run for any query with a WHERE-less OrderBy and a Take, and then ignored everything after the window. For example, OrderBy(p => p.Age).Take(10).Max(p => p.Age) returned 10 (the count), and .Any(p => p.Age > 5) returned true when no document in the window matched.
• The shortcut now applies only to the plain window: Take(n) or Skip(s).Take(n) directly on OrderBy over the collection, followed only by Select or a plain Count()/LongCount().
• Any other shape whose result depends only on the first skip + take documents is replayed through EnumerableRewriter over those documents, read from the index in order. That covers aggregates with a selector, terminals with a predicate, First, a scalar after Select, a further Skip/Take, and Take(...).Skip(...). OrderBy is a stable sort, so the replay keeps the index order and selects the same window.
• ThenBy directly after OrderBy no longer takes the shortcut, because it changes which documents fall inside the window. The same goes for a second OrderBy that the model would have picked up instead of the first.

4. EnumerableRewriter and non-generic overloads. Parameterless Sum()/Average() over int, long, double, decimal? and the rest are non-generic methods. MakeGenericMethod threw on every candidate, so Select(p => p.Age).Sum() failed with ArgumentException once (1) stopped hiding it. Non-generic candidates are now used as they are.

Tests

SelectTerminalTests runs 55 query shapes over 300 documents (Age = i % 50, with idx_Age) and compares each result with LINQ-to-Objects over the same list. 39 of them fail on main:

• Select followed by Any/All/Count(pred)/LongCount(pred)/Max/Min/Sum/Average/Contains/First(pred)/Distinct().Count(), with and without Where, Select().Select(), anonymous projections, and Sum/Average over long, double and decimal?;
• paging, then Select, then a scalar;
• the index window followed by Sum/Max with a selector, Any/Count with a predicate, Last, First, Min (descending), another Skip, Take(...).Skip(...), chained windows and ThenBy, plus Take(0);
• guards for shapes that already worked: Select().Count(), plain window lists and Where().Select().ToList().

Every Age value occurs exactly 6 times. Windows either select only Age or end on a multiple of 6, so ties at the edge of a window can't change the expected result.

Full suite on macOS: 2447 passed, 0 failed, 4 skipped. BLite.Core builds with no warnings.

Coordination

• perf: index-aware LINQ terminals with predicates, single-copy page writes, single-pass B-Tree insert (#179) #180 touches the same terminal handling. The branches merge without conflicts. On the merge, SelectTerminalTests and perf: index-aware LINQ terminals with predicates, single-copy page writes, single-pass B-Tree insert (#179) #180's QueryAndWritePathTests pass (66/66). perf: index-aware LINQ terminals with predicates, single-copy page writes, single-pass B-Tree insert (#179) #180 leaves predicates after Select on the fallback path, which is where this PR handles them.
• perf(query): LINQ Sum/Average/Min/Max from index keys or raw BSON #183 rewrites the aggregate block just above the push-down. They merge without conflicts.

Found along the way, not changed here

• The SELECT push-down never runs. TryPushDownSelect calls ScanAsync<TProj>(projector, ct) through MethodInfo.Invoke with one argument. Invoke doesn't fill in optional parameters, so it throws TargetParameterCountException, which is caught. Even if the call worked, it would return an IAsyncEnumerable<TProj>, not the IEnumerable<TProj> the caller expects. The projection is still correct through the general path, just without the push-down. I kept the method correct but didn't revive it, because perf(query): LINQ Sum/Average/Min/Max from index keys or raw BSON #183 reworks the raw-BSON scan.
• The direct pipeline keeps only the last OrderBy, Skip and Take. For example, OrderBy(p => p.Name).Take(5).OrderBy(p => p.Age) sorts all documents by Age before taking 5. A Where after Take belongs to LINQ: VisitWhere folds a Where into the root WHERE after GroupBy/OfType/SkipWhile/Take, and from a Join inner sequence #186.

🤖 Generated with Claude Code

[mrdevrobot]

Thanks. The #181 fix is right: I compared BLite with LINQ-to-Objects over four datasets (sequential, reversed ObjectIds, relocated documents, empty) for Select(...) followed by Any/Count/First/Max/Min/Sum/Average/Distinct().Count()/Contains, and for terminals after OrderBy(indexed).Take(...), including Take(0), Take(-1), Skip(1000).Take(5) and ThenBy. All match, and most were wrong on main. 39 of the 55 new tests fail on main. Merged with main, full suite 2512 / 0 / 4 skipped; with #180 too, everything passes except the #173 test I already flagged on #180. #183 is complementary.

One change: the window replay is wrong when the query uses the root queryable a second time (BTreeQueryProvider.cs:308-311, TryGetIndexWindow ~L443). EnumerableRewriter replaces every ConstantExpression equal to the root, so the second use is also replaced by the window prefix. With var q = col.AsQueryable(); over 300 documents:

query	LINQ	this PR	main
q.OrderBy(p => p.Age).Take(5).Concat(q).Count()	305	10	5
...Select(a).Concat(q.Select(a)).Count()	305	10	0
...Select(a).Union(q.Select(a)).Count()	50	1	0

Not a regression, but it breaks the invariant the PR states. In TryGetIndexWindow, return false when any argument after the window (other than Arguments[0]) contains a ConstantExpression whose value is an IQueryable; those shapes then take the general path, which already returns 305. Please add these as tests and mention the single-root requirement in the comment at ~L306.

Not for this PR:

• The replay reads QueryIndexAsync(..., 0, windowEnd), so OrderBy(idx).Skip(100000).Take(10).Sum(sel) deserializes 100,010 documents; reading skip..windowEnd and dropping the leading Skip from the replay would avoid that.
• OrderByDescending on an index returns tied keys in reverse insertion order, unlike LINQ and the non-indexed path: #210.
• While probing I found that the rewriter cache keeps captured variables from the first call (q.Count(p => p.Age > t) returns the same count for every t). It's on main too and affects the shapes this PR now routes through the rewriter: #207.

[CaffeinatedCoder]

Thanks. Addressed in three commits:

aebaaf2 is the requested change. TryGetIndexWindow returns false when an operator after the window has an argument holding an IQueryable, and the comment on the replay states the single-source requirement. New cases in SelectTerminalTests: your three (Concat(q), Select.Concat(q.Select), Select.Union(q.Select)) plus Take(5).Join(q, …). All four fail without the change (10 / 10 / 1 / 25).

515c88f: the general path had the same problem, and it is on main too. It fetches one source, filtered by the model's WHERE, and the rewriter substitutes it for every use of the root. BTreeExpressionVisitor also collects Where/Take from the other arguments of operators it doesn't model, including subqueries inside their lambdas, so one use's filter leaks into the other:

query	LINQ	before
q.Where(p => p.Age < 10).Concat(q).Count()	360	120
q.Concat(q.Where(p => p.Age < 10)).Count()	360	120
q.Where(p => p.Age == 1).Join(q, a => a.Age, b => b.Age - 1, …).Count()	36	0
q.OrderBy(p => p.Age).Take(5).Concat(q.Where(p => p.Age >= 10)).Sum(p => p.Age)	7080	7130
q.Any(x => x.Age == 0 && q.Where(y => y.Age == 49).Any())	true	false

When an operator in the main chain has a non-source argument that holds a queryable or calls a Queryable operator, the provider now reads the whole collection and replays the query. That check runs before the window path, so it replaces the one from aebaaf2. If you'd rather keep this PR narrow, aebaaf2 stands on its own, and 515c88f can move to a separate PR.

68d5e89: a query that captures q inside a lambda (x => q.Any(…)) reads it through a closure field rather than a constant, so the replay doesn't replace it. But EnumerableRewriter compiled each quoted lambda on its own, so a lambda using an outer parameter threw ("variable 'x' … is not defined"), e.g. q.Count(x => q.Any(y => y.Age == x.Age + 1)). This already failed on the general path. The window replay now hits it for OrderBy(idx).Take(n).Count(pred), where main ignored the predicate. The rewriter now leaves the lambda in the tree and compiles it with the enclosing expression.

Tests: SelectTerminalTests 70/70; full suite on the branch 2461 / 0 / 4 skipped. Merged with current main (with #180), the query tests (SelectTerminal, QueryAndWritePath, NearWithin, Linq*: 235) all pass. The full suite there showed only lock-timeout failures in concurrency tests under parallel load.

On #207: it does affect the shapes this PR sends through the rewriter. Each is right on its first run, then repeats that answer when only a captured value changes. For example, OrderBy(p => p.Age).Take(12).Count(p => p.Age > t) returns 12 for t = -1 and again for t = 0 (LINQ: 6), and Select(p => p.Age).Any(a => a == t) returns True for 7 and again for 700. On main these were wrong on every call, so it's not a regression, but the PR's results for these shapes depend on #207. It also hits correlated subqueries run through the provider: q.Where(x => q.Any(y => y.Age == x.Age + 1)).Count() gives every document the first document's answer, here 300 instead of 294. I left that case out of this PR's tests.

Skip-window cost and #210: not touched here.

[mrdevrobot]

Thanks. Re-checked on the current head merged into main (with #180): the second-source shapes (Concat(q), Select.Concat/Union(q.Select), Join(q, …)) and the general-path filter leaks now match LINQ-to-Objects, and moving the check into HasSecondaryQuery, ahead of both the window and general paths, covers more than the narrower TryGetIndexWindow guard. Keeping lambdas inline in EnumerableRewriter is fine, since the cache key and the closure behaviour are unchanged. I also probed the #180 fast paths with secondary sources (Concat(q).Count/LongCount/Sum/Max, Count(x => q…Any()), Except, Intersect, SelectMany, GroupJoin, Zip) and all match. Full suite on current main with this PR: green. The remaining mismatches I found (Take(n).Where(…) folding, #186) are pre-existing and outside this PR. Approving.