diff --git a/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs b/Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs index 213a28b52..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 @@ -628,5 +661,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..7c8b237be 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,25 @@ 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). 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; @@ -1021,46 +1031,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; + // Collect outer-to-inner so we can pick the innermost predicate's + // 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; while (source is MethodCallExpression whereCall - && QueryableVisitor.GetQueryableMethod(whereCall) == QueryableMethodKind.Where + && 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); - combined = combined == null - ? FastExpression.Lambda(rebased, param) - : FastExpression.Lambda(Expression.AndAlso(rebased, combined.Body), param); + (peeled ??= new List()).Add(predicate); + innermostWhereMethod = whereCall.Method; source = whereCall.Arguments[0]; } - return combined; + + if (peeled == null) { + return null; + } + + var param = peeled[^1].Parameters[0]; + + // 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); + body = Expression.AndAlso(body, rebased); + } + return FastExpression.Lambda(body, param); } /// diff --git a/Version.props b/Version.props index 62a69b135..3884d1feb 100644 --- a/Version.props +++ b/Version.props @@ -2,7 +2,7 @@ - 7.3.17 + 7.3.18 servicetitan