Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions Orm/Xtensive.Orm.Tests/Linq/Optimization/AggregateFusionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,39 @@ public void GroupByWhereSum_FusesIntoSingleAggregate()
Assert.That(actual, Is.EqualTo(expected));
}

/// <summary>
/// Non-fusable grouping (here: <c>Where</c>-after-<c>GroupBy</c> wraps the
/// projector with a <c>FilterProvider</c>) takes the peel-and-rebuild
/// path. <c>g.Where(p)</c> on <see cref="IGrouping{TKey, TElement}"/>
/// resolves to <see cref="Enumerable"/>.<c>Where</c>, which expects a
/// raw <c>Func</c>; quoting the lambda makes <c>Expression.Call</c>
/// throw <c>ArgumentException: Expression of type
/// 'Expression`1[Func`2[…]]' cannot be used for parameter of type
/// 'Func`2[…]'</c>.
/// </summary>
[Test]
public void NonFusableGroupingWhereSum_DoesNotThrow()
{
using var session = Domain.OpenSession();
using var tx = session.OpenTransaction();

var query = session.Query.All<Order>()
.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<Order>().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));
}
/// <summary>
/// Generalized fusion: <c>g.Where(p).Min(selector)</c> must fuse via
/// <c>g.Min(x =&gt; p(x) ? (T?)selector(x) : null)</c>; SQL <c>MIN</c>
Expand Down Expand Up @@ -628,5 +661,110 @@ public void RootLevelWhereChainSum_CollapsesAndMatchesReference()

Assert.That(actual, Is.EqualTo(expected));
}

/// <summary>
/// Inner <c>Where</c> typed for a derived entity, outer <c>Where</c>
/// typed for a wider base — the shape behind
/// <c>Owner.Items.Where(i =&gt; i.Active).Where(…).Sum(i =&gt; i.Total)</c>
/// where <c>Active</c> is declared only on the derived type.
/// <c>PeelWhereChain</c> must rebase onto the inner (narrower) parameter;
/// rebasing onto the outer parameter throws
/// <c>ArgumentException: Property '…' is not defined for type '…'</c>.
/// </summary>
[Test]
public void WhereWithDerivedTypedPredicate_CountWithBaseTypedPredicate_PreservesDerivedMemberAccess()
{
using var session = Domain.OpenSession();
using var tx = session.OpenTransaction();

var expected = session.Query.All<Order>().ToArray()
.Where(o => o.IsActive)
.Count();

// Widen via IQueryable<out T> covariance so the outer Where binds
// T = Entity while the inner Where stays Where<Order>; two peeled
// Wheres with different parameter types are required to exercise the
// rebase decision.
IQueryable<Entity> chain = session.Query.All<Order>().Where(o => o.IsActive);

var actual = chain.Where(e => e != null).Count();

Assert.That(actual, Is.EqualTo(expected));
}

/// <summary>
/// Chained generic extensions, each constrained on a different facet
/// interface (<see cref="IHasActivation"/>, <see cref="IHasCode"/>,
/// <see cref="IHasPublishDate"/>). The concrete entity satisfies every
/// constraint so the compiler binds <c>T = Order</c> throughout — a
/// regression guard ensuring the rebase fix keeps the homogeneously-typed
/// case working when interface members are accessed through an
/// <c>Order</c>-typed parameter.
/// </summary>
[Test]
public void InterfaceConstrainedExtensionsChain_Count_ResolvesAgainstConcreteEntity()
{
using var session = Domain.OpenSession();
using var tx = session.OpenTransaction();

var expected = session.Query.All<Order>().ToArray()
.Where(o => o.IsActive)
.Where(o => o.Code == "P1")
.Count(o => o.PublishedOn == new DateTime(2024, 1, 1));

var actual = session.Query.All<Order>()
.ActiveOnly()
.HavingCode("P1")
.CountPublishedOn(new DateTime(2024, 1, 1));

Assert.That(actual, Is.EqualTo(expected));
}

/// <summary>
/// Mixed-type chain: inner <c>Where&lt;Order&gt;</c> references
/// <c>Order.IsActive</c>, outer <c>Where</c>/<c>Count</c> bind
/// <c>T = IHasCode</c> after an <c>IQueryable&lt;out T&gt;</c> covariant
/// widening and reference <c>IHasCode.Code</c>. Every lambda body in the
/// peel window needs rebasing; the fix pins the accumulator to the inner
/// (<c>Order</c>) parameter so both sides' members resolve.
/// </summary>
[Test]
public void StackedMixedInterfaceWhereChain_CountWithInterfacePredicate_ResolvesViaInnermostType()
{
using var session = Domain.OpenSession();
using var tx = session.OpenTransaction();

var expected = session.Query.All<Order>().ToArray()
.Where(o => o.IsActive)
.Where(o => o.Code == "P1")
.Count(o => o.Code != "SKIP");

// Widen via IQueryable<out T> covariance; subsequent operators bind
// T = IHasCode while the inner Where stays Where<Order>.
IQueryable<IHasCode> chain = session.Query.All<Order>().Where(o => o.IsActive);

var actual = chain
.Where(x => x.Code == "P1")
.Count(x => x.Code != "SKIP");

Assert.That(actual, Is.EqualTo(expected));
}
}

/// <summary>
/// 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.
/// </summary>
internal static class FacetExtensions
{
public static IQueryable<T> ActiveOnly<T>(this IQueryable<T> q) where T : IHasActivation =>
q.Where(x => x.IsActive);

public static IQueryable<T> HavingCode<T>(this IQueryable<T> q, string code) where T : IHasCode =>
q.Where(x => x.Code == code);

public static int CountPublishedOn<T>(this IQueryable<T> q, DateTime? date) where T : IHasPublishDate =>
q.Count(x => x.PublishedOn == date);
}
}
24 changes: 23 additions & 1 deletion Orm/Xtensive.Orm.Tests/Linq/Optimization/Model.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,30 @@ public class Workflow : Entity
public string Name { get; set; }
}

/// <summary>
/// Non-intersecting facet interfaces intentionally split <see cref="Order"/>'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.
/// </summary>
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; }
Expand Down
114 changes: 80 additions & 34 deletions Orm/Xtensive.Orm/Orm/Linq/Translator.Queryable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<T_predicate> can type-mismatch
// the source when the original chain widened via IEnumerable<T>
// covariance (source is IEnumerable<Tinner> but predicate's param is
// a wider T). Quote the lambda only when the method expects an
// Expression<Func<>> (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<ColNum> columnList = null;
Expand Down Expand Up @@ -1021,46 +1031,82 @@ private CompilableProvider ChooseSourceForAggregate(CompilableProvider left, Com
}

/// <summary>
/// Walks <c>Queryable.Where</c> calls off <paramref name="source"/> and
/// combines their predicates with <c>AndAlso</c> into a single lambda.
/// Advances <paramref name="source"/> past each consumed call; leaves it
/// untouched when no <c>Where</c> is found.
/// Walks <c>Where</c> calls (<see cref="Queryable"/> or
/// <see cref="Enumerable"/>) off <paramref name="source"/> and combines
/// their predicates with <c>AndAlso</c> into a single lambda. Advances
/// <paramref name="source"/> past each consumed call; leaves it untouched
/// when no <c>Where</c> is found.
/// </summary>
/// <param name="initialParameter">
/// Optional lambda parameter to rebase every peeled predicate onto. When
/// <see langword="null"/> the first peeled predicate's own parameter is
/// adopted; subsequent predicates are rebased onto it.
/// <param name="innermostWhereMethod">
/// The <see cref="MethodInfo"/> of the innermost (last-peeled) <c>Where</c>
/// call, or <see langword="null"/> when nothing was peeled. The caller
/// can reuse it to rebuild a single <c>Where</c> node of the correct
/// flavour (Queryable vs. Enumerable) and generic argument matching the
/// peeled source's element type.
/// </param>
/// <returns>
/// The combined predicate, or <see langword="null"/> when no <c>Where</c>
/// call was peeled.
/// </returns>
private static LambdaExpression PeelWhereChain(ref Expression source, ParameterExpression initialParameter)
/// <remarks>
/// Every peeled predicate is rebased onto the innermost predicate's
/// parameter. In a well-formed LINQ <c>Where</c> chain the generic
/// argument <c>T</c> can only grow wider towards the outer calls
/// (via <see cref="IEnumerable{T}"/> covariance — <c>IQueryable{T}</c>
/// 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 …".
/// </remarks>
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<LambdaExpression> 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<T,bool>) and the indexed
// Where(source, Func<T,int,bool>) 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<T,bool>) and the
// indexed Where(src, Func<T,int,bool>) 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<LambdaExpression>()).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);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion Version.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup>
<DoVersion>7.3.17</DoVersion>
<DoVersion>7.3.18</DoVersion>
<DoVersionSuffix>servicetitan</DoVersionSuffix>
</PropertyGroup>

Expand Down
Loading