Conversation
…ueberschaer/NMF into nmf-optimizable-expressions # Conflicts: # Expressions/Expressions.Linq/EnumerableExpressions/SelectExpression.cs # Expressions/Expressions.Linq/Expressions.Linq.csproj
…ia method; comments for queryOptimizer;
…mizable-expressions
…mizable-expressions # Conflicts: # Expressions/Expressions.Linq/EnumerableExpressions/ConcatExpression.cs # Expressions/Net45/Expressions.Linq.Net45/Expressions.Linq.Net45.csproj
⚡ Risk Assessment —
|
| Files | Summary |
|---|---|
Query Optimization InfrastructureExpressions/Expressions.Linq/ExpressionsOptimization/IQueryOptimizer.cs, ProjectionMergeQueryOptimizer.cs, QueryOptimizer.cs, QueryOptimizerVisitor.cs, OptimizableExpressionExtensions.cs |
Implements query optimization framework with projection merging to combine consecutive Select/SelectMany/Join/GroupJoin expressions into single optimized expressions, reducing redundant enumeration overhead. |
Expression Interface ExtensionsExpressions/Expressions.Linq/Facade/IEnumerableExpression.cs |
Adds ISQO and IOptimizableEnumerableExpression interfaces with generic variants to enable expression classes to participate in query optimization chain. |
Select Expression OptimizationExpressions/Expressions.Linq/EnumerableExpressions/SelectExpression.cs |
Implements IOptimizableEnumerableExpression TResult, adds OptSelectorExpression/PrevExpression/OptSource properties, integrates optimizer in AsNotifiable method, adds AsOptimized/Merge methods for projection merging. |
SelectMany Expression OptimizationExpressions/Expressions.Linq/EnumerableExpressions/SelectManyExpression.cs |
Implements IOptimizableEnumerableExpression TResult, adds optimization properties and AsOptimized/Merge methods to enable projection merging with previous expressions. |
Join Expression OptimizationExpressions/Expressions.Linq/EnumerableExpressions/JoinExpression.cs |
Implements IOptimizableEnumerableExpression TResult, adds optimization properties and AsOptimized/Merge methods for join projection merging. |
GroupJoin Expression OptimizationExpressions/Expressions.Linq/EnumerableExpressions/GroupJoinExpression.cs |
Implements IOptimizableEnumerableExpression TResult, adds optimization properties and AsOptimized/Merge methods for group join projection merging. |
OrderBy/ThenBy Optimization SupportExpressions/Expressions.Linq/EnumerableExpressions/OrderByExpression.cs, ThenByExpression.cs |
Adds ISQO interface and OptSource property to enable orderby expressions to participate in optimization chains as sources. |
Test Coverage for OptimizationExpressions/Tests/Models.Expressions.Tests/NotifySystemTests.cs |
Adds test methods NotifySystem_Optimize_SelectSelect, NotifySystem_Optimize_SelectSelectSelect, NotifySystem_Optimize_SelectManySelect to verify query optimization produces correct incremental results. |
Sequence Diagram
sequenceDiagram
participant Client
participant SelectExpression
participant QueryOptimizer
participant ProjectionMergeQueryOptimizer
participant ExpressionVisitor
Client->>SelectExpression: AsNotifiable()
SelectExpression->>QueryOptimizer: Optimize<TResult>(this)
QueryOptimizer->>QueryOptimizer: Check if Source is IOptimizableEnumerableExpression
alt Source is optimizable
QueryOptimizer->>SelectExpression: Set PrevExpression = OptSelectorExpression
SelectExpression-->>QueryOptimizer: Return modified expression
QueryOptimizer->>QueryOptimizer: Recursively optimize
end
QueryOptimizer->>ProjectionMergeQueryOptimizer: MergeLambdaExpressions(prevSelector, currentSelector)
ProjectionMergeQueryOptimizer->>ExpressionVisitor: Visit(firstLambda)
ExpressionVisitor-->>ProjectionMergeQueryOptimizer: Merged lambda
ProjectionMergeQueryOptimizer-->>SelectExpression: New merged SelectExpression
SelectExpression-->>Client: INotifyEnumerable with optimized query
Dig Deeper With Commands
/review <file-path> <function-optional>/chat <file-path> "<question>"/roast <file-path>
Runs only when explicitly triggered.
|
/roast Expressions/Expressions.history |
/roast Expressions/Expressions.history
|
|
/roast Expressions/Expressions.Linq/ExpressionsOptimization/ProjectionMergeQueryOptimizer.cs |
/roast Expressions/Expressions.Linq/ExpressionsOptimization/ProjectionMergeQueryOptimizer.cs
|
|
/chat Expressions/Expressions.Linq/ExpressionsOptimization/ProjectionMergeQueryOptimizer.cs "How does the optimization work?" |
/chat Expressions/Expressions.Linq/ExpressionsOptimization/ProjectionMergeQueryOptimizer.csQ: How does the optimization work? The optimization works by merging two consecutive lambda expressions into one. The key points are:
|
|
/roast Expressions/Expressions.Linq/ExpressionsOptimization/QueryOptimizerVisitor.cs |
/roast Expressions/Expressions.Linq/ExpressionsOptimization/QueryOptimizerVisitor.cs
|
|
/chat Expressions/Expressions.Linq/ExpressionsOptimization/QueryOptimizerVisitor.cs "How does this class modify expressions?" |
/chat Expressions/Expressions.Linq/ExpressionsOptimization/QueryOptimizerVisitor.csQ: How does this class modify expressions? The |
![mergemonkeyhq[bot]](https://avatars.githubusercontent.com/in/2472657?s=60&v=4){kind=small}
| public Expression MergeLambdaExpressions(LambdaExpression firstLambdaExpression, LambdaExpression secondLambdaExpression) | ||
| { | ||
| var queryOptimizerVisitor = new QueryOptimizerVisitor(firstLambdaExpression, secondLambdaExpression); | ||
| var builtExpression = queryOptimizerVisitor.Visit(firstLambdaExpression) as LambdaExpression; |
There was a problem hiding this comment.
Merged selectors are built in the wrong direction
The projection merger walks the previous selector and substitutes its parameter with the later selector body, then reuses the later selector's parameter list. With concrete selectors like x => x.ToString() then s => s.Length, that builds the wrong expression shape instead of x => x.ToString().Length, so optimized Select chains can return the wrong projection or fail while constructing the replacement lambda.
Compose by visiting the later selector and replacing its parameter with the earlier selector body, so the merged lambda is current(previous(x)).
|
|
||
| var param = _firstLambdaSelectorExpression.Parameters[0]; | ||
|
|
||
| if (node.ToString().Equals(param.ToString())) |
There was a problem hiding this comment.
Parameter matching by ToString() rewrites the wrong variables
The optimizer identifies parameters by their printed name instead of the actual ParameterExpression instance. That breaks nested lambdas that reuse the same parameter name, because inner-scope variables get substituted even though they are different bindings.
Match parameters by reference identity, and avoid descending into nested lambdas unless their parameters are intentionally part of the substitution.
| if (expression.PrevExpression == null) | ||
| throw new ArgumentNullException("expression.PrevExpression"); | ||
|
|
||
| var mergedSelectorExpression = this.Optimize<TSource, TResult, TOptimizedResult>(expression.PrevExpression,expression.ResultSelector) as Expression<Func< TSource, TIntermediate, TOptimizedResult >>; |
There was a problem hiding this comment.
Multi-parameter selector optimization leaves unbound parameters in the merged lambda
SelectMany, Join, and GroupJoin all reuse the same one-parameter substitution routine, but their result selectors take two parameters. The merger only replaces parameter 0 and then rebuilds the lambda with the previous selector's parameter list, so the second parameter remains in the body without being declared.
Handle multi-parameter result selectors with a parameter map that preserves the untouched parameters from the later selector.
Actionable Comments Posted: 3👀 Worth a Look (1)[MEDIUM] Optimization state is stored on expression instances and never cleared - Expressions/Expressions.Linq/EnumerableExpressions/SelectExpression.cs (36, 38)🧾 Coverage Summary✔️ Covered (14 files) |
2 participants
Summary by MergeMonkey