From f149176453d5eec01fd5ef5d11126af4b156bad3 Mon Sep 17 00:00:00 2001 From: snaumenko-st Date: Fri, 24 Apr 2026 21:41:14 +0400 Subject: [PATCH 1/6] Fix: PeelWhereChain rebases onto innermost (narrowest) predicate param When aggregate fusion peels a chain of Where calls with mixed parameter types (e.g. Where(p).Where(q).Sum(...) produced by IQueryable covariance), rebasing every peeled body onto the outer widest parameter threw ArgumentException on any member that only exists on a narrower type. Rebasing onto the innermost parameter is always safe because every derived type contains all base/interface members. Made-with: Cursor --- .../Linq/Optimization/AggregateFusionTest.cs | 105 +++++++++++++++++ .../Linq/Optimization/Model.cs | 24 +++- .../Orm/Linq/Translator.Queryable.cs | 106 ++++++++++++------ 3 files changed, 202 insertions(+), 33 deletions(-) diff --git a/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs b/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs index 213a28b52..d174a90dc 100644 --- a/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs +++ b/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs @@ -628,5 +628,110 @@ public void RootLevelWhereChainSum_CollapsesAndMatchesReference() Assert.That(actual, Is.EqualTo(expected)); } + + /// + /// Inner Where typed for a derived entity, outer Where + /// typed for a wider base — the shape behind + /// Owner.Items.Where(i => i.Active).Where(…).Sum(i => i.Total) + /// where Active is declared only on the derived type. + /// PeelWhereChain must rebase onto the inner (narrower) parameter; + /// rebasing onto the outer parameter throws + /// ArgumentException: Property '…' is not defined for type '…'. + /// + [Test] + public void WhereWithDerivedTypedPredicate_CountWithBaseTypedPredicate_PreservesDerivedMemberAccess() + { + using var session = Domain.OpenSession(); + using var tx = session.OpenTransaction(); + + var expected = session.Query.All().ToArray() + .Where(o => o.IsActive) + .Count(); + + // Widen via IQueryable covariance so the outer Where binds + // T = Entity while the inner Where stays Where; two peeled + // Wheres with different parameter types are required to exercise the + // rebase decision. + IQueryable chain = session.Query.All().Where(o => o.IsActive); + + var actual = chain.Where(e => e != null).Count(); + + Assert.That(actual, Is.EqualTo(expected)); + } + + /// + /// Chained generic extensions, each constrained on a different facet + /// interface (, , + /// ). The concrete entity satisfies every + /// constraint so the compiler binds T = Order throughout — a + /// regression guard ensuring the rebase fix keeps the homogeneously-typed + /// case working when interface members are accessed through an + /// Order-typed parameter. + /// + [Test] + public void InterfaceConstrainedExtensionsChain_Count_ResolvesAgainstConcreteEntity() + { + using var session = Domain.OpenSession(); + using var tx = session.OpenTransaction(); + + var expected = session.Query.All().ToArray() + .Where(o => o.IsActive) + .Where(o => o.Code == "P1") + .Count(o => o.PublishedOn == new DateTime(2024, 1, 1)); + + var actual = session.Query.All() + .ActiveOnly() + .HavingCode("P1") + .CountPublishedOn(new DateTime(2024, 1, 1)); + + Assert.That(actual, Is.EqualTo(expected)); + } + + /// + /// Mixed-type chain: inner Where<Order> references + /// Order.IsActive, outer Where/Count bind + /// T = IHasCode after an IQueryable<out T> covariant + /// widening and reference IHasCode.Code. Every lambda body in the + /// peel window needs rebasing; the fix pins the accumulator to the inner + /// (Order) parameter so both sides' members resolve. + /// + [Test] + public void StackedMixedInterfaceWhereChain_CountWithInterfacePredicate_ResolvesViaInnermostType() + { + using var session = Domain.OpenSession(); + using var tx = session.OpenTransaction(); + + var expected = session.Query.All().ToArray() + .Where(o => o.IsActive) + .Where(o => o.Code == "P1") + .Count(o => o.Code != "SKIP"); + + // Widen via IQueryable covariance; subsequent operators bind + // T = IHasCode while the inner Where stays Where. + IQueryable chain = session.Query.All().Where(o => o.IsActive); + + var actual = chain + .Where(x => x.Code == "P1") + .Count(x => x.Code != "SKIP"); + + Assert.That(actual, Is.EqualTo(expected)); + } + } + + /// + /// Query-building extensions each constrained on a single facet interface. + /// Composed on a concrete entity that implements all three to mimic the + /// real-world pattern of pipelines assembled from small generic helpers. + /// + internal static class FacetExtensions + { + public static IQueryable ActiveOnly(this IQueryable q) where T : IHasActivation => + q.Where(x => x.IsActive); + + public static IQueryable HavingCode(this IQueryable q, string code) where T : IHasCode => + q.Where(x => x.Code == code); + + public static int CountPublishedOn(this IQueryable q, DateTime? date) where T : IHasPublishDate => + q.Count(x => x.PublishedOn == date); } } diff --git a/Orm/Xtensive.Orm.Tests/Linq/Optimization/Model.cs b/Orm/Xtensive.Orm.Tests/Linq/Optimization/Model.cs index 83d821439..97f3a6029 100644 --- a/Orm/Xtensive.Orm.Tests/Linq/Optimization/Model.cs +++ b/Orm/Xtensive.Orm.Tests/Linq/Optimization/Model.cs @@ -35,8 +35,30 @@ public class Workflow : Entity public string Name { get; set; } } + /// + /// Non-intersecting facet interfaces intentionally split 's + /// shape so that generic query-building extensions can constrain on a single + /// aspect (publishability / code-ownership / activation) without exposing + /// the full entity surface. Their member sets do not overlap — a predicate + /// typed against one cannot blindly see members declared on another. + /// + public interface IHasCode + { + string Code { get; } + } + + public interface IHasPublishDate + { + DateTime? PublishedOn { get; } + } + + public interface IHasActivation + { + bool IsActive { get; } + } + [HierarchyRoot] - public class Order : Entity + public class Order : Entity, IHasCode, IHasPublishDate, IHasActivation { [Field, Key] public long Id { get; private set; } diff --git a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs index a9f9ec98f..588040e58 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs @@ -948,7 +948,7 @@ private CompilableProvider ChooseSourceForAggregate(CompilableProvider left, Com // with an AndAlso-tree body instead of stacked FilterProviders. SQL // translation produces the same WHERE clause either way; this just // saves the RSE pipeline a few redundant nodes. - var fusionPredicate = PeelWhereChain(ref source, aggregateParameter?.Parameters[0]); + var fusionPredicate = PeelWhereChain(ref source, out var innermostWhereMethod); var fusionSelector = aggregateParameter; if (aggregateType == AggregateType.Count) { // Count's own predicate participates as part of the fusion predicate, @@ -972,15 +972,19 @@ private CompilableProvider ChooseSourceForAggregate(CompilableProvider left, Com } else if (fusionPredicate != null) { // Non-fusable Sum/Min/Max/Avg with peeled Where(s): rebuild source as - // a single Queryable.Where(source, fusionPredicate) so the primary + // a single Where(source, fusionPredicate) call so the primary // aggregate-selector path below sees one FilterProvider instead of // stacked ones. Equivalent SQL in either form (both emit - // WHERE f1 AND ... AND fn on the same SELECT), but keeps the RSE - // tree smaller and consistent with the non-fusable Count path. - source = Expression.Call(WellKnownMembers.Queryable.Where.CachedMakeGenericMethod( - fusionPredicate.Parameters[0].Type), - source, - Expression.Quote(fusionPredicate)); + // WHERE f1 AND ... AND fn on the same SELECT); this just keeps the + // RSE tree smaller and consistent with the non-fusable Count path. + // + // Reuses the innermost peeled Where's MethodInfo so we preserve the + // original Queryable.Where/Enumerable.Where choice and exact generic + // arg — hard-coding Queryable.Where can type-mismatch + // the source when the original chain widened via IEnumerable + // covariance (source is IEnumerable but predicate's param is + // a wider T). + source = Expression.Call(innermostWhereMethod, source, Expression.Quote(fusionPredicate)); } IReadOnlyList columnList = null; @@ -1021,44 +1025,82 @@ private CompilableProvider ChooseSourceForAggregate(CompilableProvider left, Com } /// - /// Walks Queryable.Where calls off and - /// combines their predicates with AndAlso into a single lambda. - /// Advances past each consumed call; leaves it - /// untouched when no Where is found. + /// Walks Where calls ( or + /// ) off and combines + /// their predicates with AndAlso into a single lambda. Advances + /// past each consumed call; leaves it untouched + /// when no Where is found. /// - /// - /// Optional lambda parameter to rebase every peeled predicate onto. When - /// the first peeled predicate's own parameter is - /// adopted; subsequent predicates are rebased onto it. + /// + /// The of the innermost (last-peeled) Where + /// call, or when nothing was peeled. The caller + /// can reuse it to rebuild a single Where node of the correct + /// flavour (Queryable vs. Enumerable) and generic argument matching the + /// peeled source's element type. /// /// /// The combined predicate, or when no Where /// call was peeled. /// - private static LambdaExpression PeelWhereChain(ref Expression source, ParameterExpression initialParameter) + /// + /// Every peeled predicate is rebased onto the innermost predicate's + /// parameter. In a well-formed LINQ Where chain the generic + /// argument T can only grow wider towards the outer calls + /// (via covariance — IQueryable{T} + /// is invariant so widening happens only on the Enumerable boundary), + /// which makes the innermost parameter the narrowest type across the + /// chain. A subtype has every member of all its supertypes, so rebasing + /// every outer body onto it preserves the member accesses inside. The + /// reverse (rebasing onto an outer/wider parameter) would produce + /// member-access expressions that reference members not declared on + /// the rebased parameter's static type and fail downstream translation + /// with "property not defined for type …". + /// + private static LambdaExpression PeelWhereChain(ref Expression source, out MethodInfo innermostWhereMethod) { - LambdaExpression combined = null; - var param = initialParameter; - while (source is MethodCallExpression whereCall - && QueryableVisitor.GetQueryableMethod(whereCall) == QueryableMethodKind.Where + // Collect outer-to-inner so we can pick the innermost predicate's + // parameter (narrowest type) to rebase everyone onto. + List peeled = null; + innermostWhereMethod = null; + var current = source; + while (current is MethodCallExpression whereCall + && GetQueryableMethod(whereCall) == QueryableMethodKind.Where && whereCall.Arguments.Count == 2) { - var predicate = (LambdaExpression) whereCall.Arguments[1].StripQuotes(); - // Queryable exposes both Where(source, Func) and the indexed - // Where(source, Func) under the same QueryableMethodKind. - // Only the 1-parameter overload is safe to AND-combine: the indexed - // variant binds the element's position in the *current* sequence, so - // collapsing it into another Where would change the index semantics. - // Bail out and let the caller leave the indexed Where in place for - // the normal VisitWhere path to handle. + var predicate = whereCall.Arguments[1].StripQuotes(); + // Queryable/Enumerable both expose Where(src, Func) and the + // indexed Where(src, Func) under the same + // QueryableMethodKind. Only the 1-parameter overload is safe to + // AND-combine: the indexed variant binds the element's position in + // the *current* sequence, so collapsing it into another Where would + // change the index semantics. Stop peeling and let the caller hand + // the remaining chain to the normal VisitWhere path. if (predicate.Parameters.Count != 1) { break; } - param ??= predicate.Parameters[0]; - var rebased = ExpressionReplacer.Replace(predicate.Body, predicate.Parameters[0], param); + (peeled ??= new List()).Add(predicate); + innermostWhereMethod = whereCall.Method; + current = whereCall.Arguments[0]; + } + + if (peeled == null) { + return null; + } + + source = current; + var param = peeled[^1].Parameters[0]; + + // Iterate outer-to-inner so the resulting AndAlso tree keeps the + // outer predicate on the outer branch; short-circuit semantics then + // evaluate inner-most predicate first, matching the original nested + // Where order. + LambdaExpression combined = null; + foreach (var pred in peeled) { + var rebased = ReferenceEquals(pred.Parameters[0], param) + ? pred.Body + : ExpressionReplacer.Replace(pred.Body, pred.Parameters[0], param); combined = combined == null ? FastExpression.Lambda(rebased, param) : FastExpression.Lambda(Expression.AndAlso(rebased, combined.Body), param); - source = whereCall.Arguments[0]; } return combined; } From be76b9fda3a7c6df6220eaa933872cf882a85637 Mon Sep 17 00:00:00 2001 From: Sergey Naumenko <152863015+snaumenko-st@users.noreply.github.com> Date: Fri, 24 Apr 2026 21:45:10 +0400 Subject: [PATCH 2/6] Update DoVersionSuffix for testing purposes --- Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Version.props b/Version.props index 62a69b135..8378df22f 100644 --- a/Version.props +++ b/Version.props @@ -3,7 +3,7 @@ 7.3.17 - servicetitan + servicetitan-test-1 From e21281eac2df746d62fb5f988e3aa1377d90ab13 Mon Sep 17 00:00:00 2001 From: snaumenko-st Date: Fri, 24 Apr 2026 23:47:36 +0400 Subject: [PATCH 3/6] Fix: skip Expression.Quote when rebuilding Enumerable.Where in aggregate peel The non-fusable Sum/Min/Max/Avg peel path rebuilt the Where call with the predicate unconditionally wrapped in Expression.Quote. That is correct for Queryable.Where (parameter type is Expression>) but throws ArgumentException for Enumerable.Where (parameter type is Func<>), which is what g.Where(p) on an IGrouping resolves to. Quote only when the captured innermost Where method expects an Expression-derived parameter. Made-with: Cursor --- .../Linq/Optimization/AggregateFusionTest.cs | 33 +++++++++++++++++++ .../Orm/Linq/Translator.Queryable.cs | 10 ++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs b/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs index d174a90dc..49a35026a 100644 --- a/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs +++ b/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs @@ -440,6 +440,39 @@ public void GroupByWhereSum_FusesIntoSingleAggregate() Assert.That(actual, Is.EqualTo(expected)); } + /// + /// Non-fusable grouping (here: Where-after-GroupBy wraps the + /// projector with a FilterProvider) takes the peel-and-rebuild + /// path. g.Where(p) on + /// resolves to .Where, which expects a + /// raw Func; quoting the lambda makes Expression.Call + /// throw ArgumentException: Expression of type + /// 'Expression`1[Func`2[…]]' cannot be used for parameter of type + /// 'Func`2[…]'. + /// + [Test] + public void NonFusableGroupingWhereSum_DoesNotThrow() + { + using var session = Domain.OpenSession(); + using var tx = session.OpenTransaction(); + + var query = session.Query.All() + .GroupBy(o => o.IsActive) + .Where(g => g.Key) + .Select(g => new { + Active = g.Key, + PublishedSum = g.Where(x => x.PublishedOn != null).Sum(x => x.Id), + }); + + var expected = session.Query.All().ToArray() + .GroupBy(o => o.IsActive) + .Where(g => g.Key) + .Select(g => (g.Key, PublishedSum: g.Where(x => x.PublishedOn != null).Sum(x => x.Id))) + .ToArray(); + + var actual = query.ToArray().Select(r => (r.Active, r.PublishedSum)).ToArray(); + Assert.That(actual, Is.EquivalentTo(expected)); + } /// /// Generalized fusion: g.Where(p).Min(selector) must fuse via /// g.Min(x => p(x) ? (T?)selector(x) : null); SQL MIN diff --git a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs index 588040e58..419c7f836 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs @@ -983,8 +983,14 @@ private CompilableProvider ChooseSourceForAggregate(CompilableProvider left, Com // arg — hard-coding Queryable.Where can type-mismatch // the source when the original chain widened via IEnumerable // covariance (source is IEnumerable but predicate's param is - // a wider T). - source = Expression.Call(innermostWhereMethod, source, Expression.Quote(fusionPredicate)); + // a wider T). Quote the lambda only when the method expects an + // Expression> (Queryable.Where); Enumerable.Where (used on + // IGrouping/IEnumerable sources) takes a raw Func<> directly. + var whereParamType = innermostWhereMethod.GetParameters()[1].ParameterType; + var predicateArg = typeof(Expression).IsAssignableFrom(whereParamType) + ? (Expression) Expression.Quote(fusionPredicate) + : fusionPredicate; + source = Expression.Call(innermostWhereMethod, source, predicateArg); } IReadOnlyList columnList = null; From 61bf4c6be32756e94483360bf46079b5fe770afc Mon Sep 17 00:00:00 2001 From: snaumenko-st Date: Sat, 25 Apr 2026 01:39:09 +0400 Subject: [PATCH 4/6] Refactor: simplify PeelWhereChain combine loop Seed the AndAlso accumulator from peeled[^1].Body (already typed against the rebase parameter) and iterate only the outer predicates that need rebasing. Drops the per-iteration null check and reduces FastExpression.Lambda allocations from one-per-predicate to one total. Made-with: Cursor --- .../Orm/Linq/Translator.Queryable.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs index 419c7f836..ab67d1dbd 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs @@ -1095,20 +1095,18 @@ private static LambdaExpression PeelWhereChain(ref Expression source, out Method source = current; var param = peeled[^1].Parameters[0]; - // Iterate outer-to-inner so the resulting AndAlso tree keeps the - // outer predicate on the outer branch; short-circuit semantics then - // evaluate inner-most predicate first, matching the original nested - // Where order. - LambdaExpression combined = null; - foreach (var pred in peeled) { + // Build (p_inner AND p_next AND ... AND p_outer) so short-circuit + // evaluation matches the original nested Where order. peeled[^1] is + // already typed against param, so only outer predicates need rebasing. + var body = peeled[^1].Body; + for (var i = peeled.Count - 2; i >= 0; i--) { + var pred = peeled[i]; var rebased = ReferenceEquals(pred.Parameters[0], param) ? pred.Body : ExpressionReplacer.Replace(pred.Body, pred.Parameters[0], param); - combined = combined == null - ? FastExpression.Lambda(rebased, param) - : FastExpression.Lambda(Expression.AndAlso(rebased, combined.Body), param); + body = Expression.AndAlso(body, rebased); } - return combined; + return FastExpression.Lambda(body, param); } /// From bed944585e10bceed0b8be37a0a35f03d9ecfb01 Mon Sep 17 00:00:00 2001 From: snaumenko-st Date: Sat, 25 Apr 2026 01:47:11 +0400 Subject: [PATCH 5/6] Refactor: drop `current` local in PeelWhereChain, advance `source` in place source is already a ref Expression; the local copy and final write-back were redundant. The bail-out branches naturally leave source at the unconsumed remainder. Made-with: Cursor --- Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs index ab67d1dbd..7c8b237be 100644 --- a/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs +++ b/Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs @@ -1065,11 +1065,12 @@ private CompilableProvider ChooseSourceForAggregate(CompilableProvider left, Com private static LambdaExpression PeelWhereChain(ref Expression source, out MethodInfo innermostWhereMethod) { // Collect outer-to-inner so we can pick the innermost predicate's - // parameter (narrowest type) to rebase everyone onto. + // parameter (narrowest type) to rebase everyone onto. source is + // advanced in place; an indexed-Where bail or non-Where node leaves + // it pointing at the unconsumed remainder of the chain. List peeled = null; innermostWhereMethod = null; - var current = source; - while (current is MethodCallExpression whereCall + while (source is MethodCallExpression whereCall && GetQueryableMethod(whereCall) == QueryableMethodKind.Where && whereCall.Arguments.Count == 2) { var predicate = whereCall.Arguments[1].StripQuotes(); @@ -1085,14 +1086,13 @@ private static LambdaExpression PeelWhereChain(ref Expression source, out Method } (peeled ??= new List()).Add(predicate); innermostWhereMethod = whereCall.Method; - current = whereCall.Arguments[0]; + source = whereCall.Arguments[0]; } if (peeled == null) { return null; } - source = current; var param = peeled[^1].Parameters[0]; // Build (p_inner AND p_next AND ... AND p_outer) so short-circuit From c2525a02041c1b0a4ccbdb3e94c66f117525bbce Mon Sep 17 00:00:00 2001 From: Sergey Naumenko <152863015+snaumenko-st@users.noreply.github.com> Date: Sat, 25 Apr 2026 11:47:42 +0400 Subject: [PATCH 6/6] Update Version.props --- Version.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Version.props b/Version.props index 75bf3f1e0..3884d1feb 100644 --- a/Version.props +++ b/Version.props @@ -2,8 +2,8 @@ - 7.3.17 - servicetitan-test-2 + 7.3.18 + servicetitan