Uh oh!
There was an error while loading. Please reload this page.
[release/11.0] Lift mixed outer-reference aggregate arguments - #38925
Open
AndriySvyryd with Copilot wants to merge 1 commit into
Open
[release/11.0] Lift mixed outer-reference aggregate arguments#38925AndriySvyryd with Copilot wants to merge 1 commit into
AndriySvyryd with Copilot wants to merge 1 commit into
Conversation
- Track, per aggregate invocation, whether the argument references a column of the SELECT the aggregate is evaluated in (local) and whether it references a column from further out (outer), and lift the argument when both are present. The postprocessor only lifted arguments containing a subquery, so these expressions reached the server unmodified and failed with "Multiple columns are specified in an aggregated expression containing an outer reference" - Rename SqlServerAggregateOverSubqueryPostprocessor to SqlServerAggregateArgumentPostprocessor, since a subquery in the argument is no longer the only thing that triggers a lift - Compare against the aggregating SELECT's own table aliases rather than _tableAliasesInScope: that set deliberately excludes those tables, since a lifted subquery reaches them through APPLY rather than CROSS JOIN - Leave an aggregate whose argument is purely an outer reference alone; SQL Server evaluates it in the outer query, which is how aggregates over an outer grouping are meant to translate - Restore the parent visitor state before building the lifted subquery, and capture _isCorrelatedSubquery for the OUTER APPLY/CROSS JOIN choice, so the state saved by an enclosing aggregate is no longer clobbered - Add specification tests for a Sum over an expression with an outer reference and for Sums over members of a single-result subquery, plus SQL Server baselines Fixes#38834 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
CopilotAI
changed the title
[release/11.0] Lift aggregate arguments that mix an outer reference with a local column[release/11.0] Lift mixed outer-reference aggregate argumentsSep 5, 2026
AndriySvyryd
marked this pull request as ready for review
September 5, 2026 01:15
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new relational spec test dereferences a nullable FirstOrDefault() result (d.OrderID) and will fail the build due to warnings-as-errors.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses a SQL Server limitation where aggregate arguments that mix an outer reference with a local column (SQL error 8124) produce invalid SQL, by detecting that pattern during postprocessing and lifting the aggregate argument via OUTER APPLY/CROSS JOIN. It also adds regression coverage for both the direct mixed-reference case and the single-result-subquery projection case.
Changes:
- Extend/rename the SQL Server aggregate argument postprocessor to lift arguments that contain both local and outer references (in addition to the existing “contains subquery” trigger).
- Fix visitor state handling for nested aggregates by restoring parent state before performing the lift while capturing correlation state.
- Add relational spec tests and SQL Server baselines covering mixed outer/local aggregate arguments and the single-result-subquery member projection regression.
File summaries
| File | Description |
|---|---|
| test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs | Adds SQL Server baselines asserting the lifted OUTER APPLY shape for the new regression tests. |
| test/EFCore.Relational.Specification.Tests/Query/NorthwindAggregateOperatorsQueryRelationalTestBase.cs | Adds two new relational spec tests covering mixed outer-reference aggregate arguments (direct and via projected single-result subquery). |
| src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs | Switches to the renamed/extended aggregate argument postprocessor. |
| src/EFCore.SqlServer/Query/Internal/SqlServerAggregateArgumentPostprocessor.cs | Implements mixed local/outer reference detection and lifts aggregate arguments to avoid SQL Server error 8124. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| select new | ||
| { | ||
| Products = d!.ProductID * c.CustomerID.Length, | ||
| Orders = d.OrderID * c.CustomerID.Length |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#38834
Port of #38855
SQL Server rejects aggregate expressions combining an outer reference with a local column, causing valid LINQ queries to fail with error 8124.
OUTER APPLYorCROSS JOIN.