Lift aggregate arguments that mix an outer reference with a local column

[benedict-odonovan]

Fixes #38834

• I've read the guidelines for contributing and seen the walkthrough
• I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
• The code builds and tests pass locally (also verified by our automated build checks)
• Commit messages follow this format
• Tests for the changes have been added (for bug fixes / features)
• Code follows the same patterns and style as existing code in this repo

Summary

SqlServerAggregateArgumentPostprocessor (renamed from SqlServerAggregateOverSubqueryPostprocessor in this PR) works around SQL Server's refusal to aggregate over a subquery by lifting the aggregate's argument out to an OUTER APPLY/CROSS JOIN and aggregating the resulting column. SQL Server has a second restriction on aggregates which the postprocessor didn't cover:

Msg 8124: Multiple columns are specified in an aggregated expression containing an outer reference. If an expression being aggregated contains an outer reference, then that outer reference must be the only column referenced in the expression.

An aggregate argument that combines a column of the SELECT the aggregate is evaluated in with a column from further out runs into this. The postprocessor only lifted arguments in which a subquery was found, so such an expression reached the server unchanged and the query failed at execution.

The reported shape sums, per customer, a value that multiplies a column read from a single-result subquery by a column on the outer entity:

from c in ss.Set<Customer>()
let projected = from o in c.Orders
                let d = o.OrderDetails.OrderBy(od => od.ProductID)
                    .Select(od => new { od.ProductID, od.OrderID })
                    .FirstOrDefault()
                select new
                {
                    Products = d!.ProductID * c.CustomerID.Length,
                    Orders = d.OrderID * c.CustomerID.Length
                }
select new
{
    c.CustomerID,
    TotalProducts = projected.Sum(x => x.Products),
    TotalOrders = projected.Sum(x => x.Orders)
}

-- before: [c].[CustomerID] is an outer reference and [o2].[ProductID] is a second
-- column in the same aggregated expression; SQL Server rejects this with error 8124
SELECT [c].[CustomerID], (
    SELECT ISNULL(SUM([o2].[ProductID] * CAST(LEN([c].[CustomerID]) AS int)), 0)
    FROM [Orders] AS [o]
    LEFT JOIN (
        SELECT [o1].[ProductID], [o1].[OrderID0]
        FROM (
            SELECT [o0].[ProductID], [o0].[OrderID] AS [OrderID0], ROW_NUMBER() OVER(PARTITION BY [o0].[OrderID] ORDER BY [o0].[ProductID]) AS [row]
            FROM [Order Details] AS [o0]
        ) AS [o1]
        WHERE [o1].[row] <= 1
    ) AS [o2] ON [o].[OrderID] = [o2].[OrderID0]
    WHERE [c].[CustomerID] = [o].[CustomerID]) AS [TotalProducts], ...

-- after: the multiplication is lifted into an OUTER APPLY, so the SUM sees one column
SELECT [c].[CustomerID], (
    SELECT ISNULL(SUM([s].[value]), 0)
    FROM [Orders] AS [o]
    LEFT JOIN (
        SELECT [o1].[ProductID], [o1].[OrderID0]
        FROM (
            SELECT [o0].[ProductID], [o0].[OrderID] AS [OrderID0], ROW_NUMBER() OVER(PARTITION BY [o0].[OrderID] ORDER BY [o0].[ProductID]) AS [row]
            FROM [Order Details] AS [o0]
        ) AS [o1]
        WHERE [o1].[row] <= 1
    ) AS [o2] ON [o].[OrderID] = [o2].[OrderID0]
    OUTER APPLY (
        SELECT [o2].[ProductID] * CAST(LEN([c].[CustomerID]) AS int) AS [value]
    ) AS [s]
    WHERE [c].[CustomerID] = [o].[CustomerID]) AS [TotalProducts], ...

This is a regression in 11.0 rc.1 relative to preview.6, but the bug is older than that and isn't in the postprocessor — what changed is the shape of the argument reaching it. Before #38502, a single-result subquery projecting two or more members was repeated once per member, so the aggregate argument still contained a ScalarSubqueryExpression and the existing subquery trigger fired, lifting the outer reference out along with it. #38502 replaces those repeated subqueries with a single join; with no subquery left in the argument, nothing triggers the lift and the outer reference stays inside the aggregate. That's also why the workaround in the issue — projecting a single member — keeps the query working.

The same failure is reachable without any subquery at all, e.g. o.OrderDetails.Sum(od => od.ProductID * o.OrderID), which is the second test added here.

Implementation

The class is renamed from SqlServerAggregateOverSubqueryPostprocessor to SqlServerAggregateArgumentPostprocessor, since a subquery in the argument is no longer the only thing that makes it lift. It's an internal-API type under Query/Internal, and SqlServerQueryTranslationPostprocessor is its only reference.

SqlServerAggregateArgumentPostprocessor now tracks two more facts per aggregate invocation, alongside the existing "argument contains a subquery":

• local reference — the argument references a column of the SELECT the aggregate is evaluated in
• outer reference — the argument references a column from further out

and lifts the argument when both are present, in addition to the existing subquery trigger. Everything downstream of the trigger — building the lifted subquery, choosing OUTER APPLY vs CROSS JOIN, rewriting the aggregate to read the lifted projection — is reused unchanged.

Both facts are recorded in the existing ColumnExpression case, which already fires for exactly the columns of interest: those not in _tableAliasesInScope. Classification compares against the aggregating SELECT's own table aliases (_currentSelect.Tables) rather than _tableAliasesInScope; that set deliberately excludes those tables, since a lifted subquery has to reach them through APPLY rather than CROSS JOIN, so it can't tell "column of the aggregating SELECT" from "column of an enclosing one".

An argument that is purely an outer reference is deliberately left alone: SQL Server accepts it and evaluates the aggregate in the outer query, which is how aggregates over an outer grouping are meant to translate. Only the mixed case is rejected by the server, and only there is lifting both necessary and unambiguous.

One pre-existing bug surfaced along the way: the parent visitor state was restored after the lifted subquery was built, so a nested aggregate clobbered the state saved by an enclosing one, and _isCorrelatedSubquery — read to choose between OUTER APPLY and CROSS JOIN — could reflect the wrong invocation. The restore now happens before the lift, with the flag captured into a local first.

Testing

Two new specification tests in NorthwindAggregateOperatorsQueryRelationalTestBase, with SQL Server baselines:

Test	Shape
Sum_over_expression_with_outer_reference	Sum over an expression multiplying a column of the aggregated collection by a column of the outer entity — the minimal form, no subquery involved
Sum_over_members_of_single_result_subquery_with_outer_reference	the reported shape: two Sums over members of a FirstOrDefault() subquery, each combined with an outer column

Both fail against main with SQL Server error 8124 and pass with this change; both also run green on SQLite through the shared base.

[Copilot]

Pull request overview

This PR extends the SQL Server aggregate-argument lifting postprocessor to also handle aggregate arguments that mix an outer reference with a local column (SQL Server error 8124), not just subqueries, by lifting the computed expression into an OUTER APPLY/CROSS JOIN. It also renames the postprocessor to reflect the broader responsibility and adds coverage for the new translation shape.

Changes:

• Rename SqlServerAggregateOverSubqueryPostprocessor to SqlServerAggregateArgumentPostprocessor and expand lifting triggers to include mixed outer+local references.
• Fix visitor state restoration ordering to avoid nested aggregates clobbering correlation state used to choose OUTER APPLY vs CROSS JOIN.
• Add two new relational spec tests and SQL Server baselines validating the new lifting behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs	Adds SQL Server baselines for the two new aggregate/outer-reference test cases.
test/EFCore.Relational.Specification.Tests/Query/NorthwindAggregateOperatorsQueryRelationalTestBase.cs	Adds new spec tests reproducing SQL Server error 8124 shapes (with and without subqueries).
src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs	Switches to the renamed aggregate argument postprocessor.
src/EFCore.SqlServer/Query/Internal/SqlServerAggregateArgumentPostprocessor.cs	Implements mixed outer+local reference detection, lifting logic trigger, and fixes nested-aggregate visitor state handling.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.