Repository navigation
Stop producing duplicate subqueries in SQL #32277
Description
Here's a thought dump on SQL duplication. There's no definite proposal yet below, this requires more thinking; ultimately, this may be more about how we translate GroupBy and handle pending selectors.
The following LINQ query (see #32234 (comment) for the full repro):
_ = await context.IterationValues
.Where(r => ids.Contains(r.IterationId))
.GroupBy(x => x.IterationId)
.Select(x => new
{
x.Key,
MaxTimestamp = x.Select(x => x.Iteration.Timestamp).Max(),
})
.OrderBy(x => x.MaxTimestamp)
.ToListAsync();... produces the following SQL:
SELECT [i].[IterationId] AS [Key], (
SELECT MAX([i5].[Timestamp])
FROM [IterationValues] AS [i4]
INNER JOIN [Iteration] AS [i5] ON [i4].[IterationId] = [i5].[Id]
WHERE [i4].[IterationId] IN (
SELECT [i6].[value]
FROM OPENJSON(@__ids_0) WITH ([value] int '$') AS [i6]
) AND [i].[IterationId] = [i4].[IterationId]) AS [MaxTimestamp]
FROM [IterationValues] AS [i]
WHERE [i].[IterationId] IN (
SELECT [i0].[value]
FROM OPENJSON(@__ids_0) WITH ([value] int '$') AS [i0]
)
GROUP BY [i].[IterationId]
ORDER BY (
SELECT MAX([i5].[Timestamp])
FROM [IterationValues] AS [i4]
INNER JOIN [Iteration] AS [i5] ON [i4].[IterationId] = [i5].[Id]
WHERE [i4].[IterationId] IN (
SELECT [i3].[value]
FROM OPENJSON(@__ids_0) WITH ([value] int '$') AS [i3]
) AND [i].[IterationId] = [i4].[IterationId])There is considerable subquery duplication in the above SQL: the ordering and projection contains the exact same subquery, and the OPENJSON subquery contained within it is also duplicate in the WHERE clause. This is quite inefficient, causing the database to evaluate the subquery multiple times.
- One eager approach here would be to perform a pushdown in translation, when an operator is being applied that would cause duplication. For example, with the above query, when the OrderBy is applied, we'd detect that the lambda references a SelectExpression that's also in the projection, do a pushdown, and then apply the ordering on top of that. This has the disadvantage of being probably too eager: if a further Select is then applied which removes the subquery from the projection, the pushdown is no longer necessary, but we already did it too early. Also, finding out whether the OrderBy references the projection isn't trivial (either an additional visitation on all lambdas just to detect this, or to change the way we translate lambdas, passing them more context).
- Another approach would be to add a phase in post-processing, where we find all SelectExpressions which are referenced multiple times, and extract those out to CTEs; this would add one pass for discovery, and another one to apply the transformation if needed (to optimize this we may gather the discovery information as part of translation and pass it forward to post-processing). There are probably some other CTE-related post-processing needs, so this visitor probably makes sense as a new post-processing step.
- For the ordering and projection duplication above, a CTE may be slightly less great than a simple subquery pushdown (even if perf-wise they should be the same). We could detect the possibility to push down instead of introduce a CTE (because the duplication is between the projection and another clause, i.e. ordering), but that may be too much trouble for what its worth.
- Note that this second approach needs to happen right after translation, before type mapping inference visitation happens - or any other visit which may rewrite the tree. This is because such a visitor may cause the SelectExpression to get duplicated, and we lose the multiple referencing.
However, note that the WHERE clause itself also gets duplicated in the above ordering and projection. The ideal way to rewrite this query is to perform a pushdown right after the WHERE clause, so that the IN operator is only evaluated once. That does point towards approach (1), i.e. detecting that the projection contains a complex, potentially expensive expression which is better off being pushed down.
Activity
Charlieface commentedon Jun 24, 2025
This is definitely something that continually pops up in more complex querying. Workarounds including manual pushdown, and many queries needs babysitting to check over the SQL and avoid this issue.
Solution #1 seems much better to me. But,
I think a better solution might be to explore a new style of pushdown for calculations: using a
lateraljoin (orCROSS APPLYfor SQL Server), with a single-row correlatedVALUESorSELECTclause. So the above query becomesThe other benefit of such processing is that this doesn't only need to happen with subqueries, we could do it with any complex expression. This avoids all sorts of repetition of calculations in
WHEREORDER BYandSELECT: by moving them into a lateral join, you can reference them like any other column, and the database optimizer can understand that they're identical. I wrote a post on StackOverflow about this technique. So any scalar or subquery expression used multiple times in the query gets pushed into a lateral, and only appears once in the query.The benefit of a lateral join here rather than full pushdown is that it's much less verbose, so the downside is less. It's also much easier to remove in post-processing: if we discover a lateral is only used once. We can simply remove it and replace the column ref with the actual calculation.
This is a complete non-starter.
In both Postgres and SQL Server, a CTE is no different to a view and fully inlined, so the performance will be identically bad to the original. You could post-process and use pushdown or lateral join as above, but you might as well do that in the main translation phase.
Also, a CTE is only going to help for a non-correlated subquery. You can't CTE your way out of a correlated subquery, because the correlation isn't in scope in the CTE.
roji commentedon Jun 24, 2025
Thanks for chiming in.
Since posting this issue, I think it has become clear that the real problem to be fixed here is the moving around of Select() that happens in EF's preprocessing pipeline (AKA the "pending selector" behavior of the navigation expansion visitor); see #20291. This effectively moves Select() operators as far forward as possible; in the query above, it switches the order of the Select and the OrderBy. Since OrderBy refers to a subquery projected by the Select (represented by MaxTimestamp), the subquery gets duplicated. We should stop doing that: at that point the Select would get translated in the place that its at without moving forward, and any further processing on top wouldn't duplicate it.
So at this point I'm pretty sure that this issue is a duplicate of #20291, but will keep this open to make sure that this scenario is properly handled once #20291 is done. This is one of EF's high-priority query performance issues, and is near the top of my list (though it is not happening for EF 10 unfortunately).
This is not true. At the very least for PostgreSQL, CTEs may be materialized and therefore get evaluated once even when being referred to multiple times - see these PG docs. I'm not familiar with how SQL Server deals with CTEs - do you have a resource confirming what you're saying above?
Charlieface commentedon Jun 25, 2025
You're quite right, I missed that even though Postgres doesn't materialize by default, it does when there are multiple references.
SQL Server - this is a well known fact that CTEs, views and derived tables aren't materialized. See also this post by a Microsoft employee. A pull request has just been made and the docs are now updated to explain this.
Notes for other DBMSs:
roji commentedon Jun 25, 2025
Thanks for that overview of the materialization behavior across databases! I'm indeed a bit surprised to not see materialization more of a well-supported thing that happens default (when appropriate). In some cases I'd also expect more intelligence in exactly when to materialize - the fact that there are multiple references might not necessarily mean that it's worth materializing (depending on the internal complexity of the CTE).
In any case, just to be clear - I indeed agree with your earlier statement that we shouldn't use (or need to use) CTEs to fix the specific problem here; we should simply stop moving the Select() forward and produce SQL that uses subqueries appropriate to evaluate just once.
bachratyg commentedon Jun 25, 2025
FYI that points to MySQL docs.
bachratyg commentedon Jun 26, 2025
Additional unofficial Oracle documentation here.
My personal experience is that the optimizer always inlines single-ref CTEs and though very much capable of inlining multi-ref without a hint chooses not to do so except for the most basic cases e.g. select-from projection without a where clause. So there's no reliable default.
roji commentedon Jun 26, 2025
I honestly see very little reason not to inline single-ref CTEs - there's a non-trivial cost associated with materializing and that really has no benefit I can see for that case. For multi-ref CTEs I'd expect a good planner to make a decision based on the CTE complexity, possibly erring on the side of materializing rather than inlining - but those are just expectations, and not any indications of what I've seen in the real world.
Note that there are multiple issues for CTE support in EF (#29918, #26486), but those are more about providing the user with ways to express a CTE. #29918 in particular is interesting: when an IQueryable local variable is referenced in an EF LINQ query, it is currently inlined early in the query pipeline (similar to how database inline CTEs); #29918 would be about translating it as a CTE instead (though EF could also inline that if it's single-ref).
ranma42 commentedon Jun 27, 2025
I am not sure if this is guaranteed by all implementations, but I would expect CTEs to have consistent values (specifically in the case of multi-ref). This might be a way to tackle issues with re-evaluation of impure expressions:
roji commentedon Jun 27, 2025
@ranma42 that's a very good point... The PG docs explicitly state:
So function purity indeed plays a role in the decision whether to inline (but this can be overridden in SQL via a hint).
Sure, though this is limited only to rowset contexts (since CTEs represent a rowset), whereas we have the impurity problem also in scalar contexts (e.g. when we duplicate a function call in a WHERE clause because of null semantics) - not sure that's going to be helpful there.
Charlieface commentedon Jun 27, 2025
@roji
SQL Server's optimizer doesn't work this way at all. It folds views, derived tables, CTEs and inline Table Valued Functions directly into the query possibly multiple times. At this point the planner can't see any difference between any of those constructs. Then while planning, it makes decisions as it does in many cases about whether to use an Index Spool, this is unrelated to how that part of the query is actually represented in SQL.
But whether or not it spools, if a CTE is referenced multiple times then it may get spooled multiple times separately, as the optimizer can't see that they are the same. You can see an example using this fiddle, for which I used some tricks to force a Row-Count Index Spool. You can clearly see two spools for one CTE.
@ranma42
Again, SQL Server just doesn't work this way. Because a CTE and a view are simply inlined, there are multiple references to side-effecting operators such as
NEWID(). So you can easily get invalid results even with CTEs. In the majority of cases in SQL Server, the only real solution is to lift out the side-affecting operator into a variable, and for multiple rows to do a bareINSERT... SELECTto a table variable/temp table. CTEs just don't work here.roji commentedon Jun 28, 2025
Yep, thanks for the dbfiddle sample showing this - that's IMHO quite unfortunate behavior. Here's the same thing in PostgreSQL, showing that the CTE is not inlined, but rather materialized.
ranma42 commentedon Jun 30, 2025
💔 just like for
COALESCE(see the docs; related to the work towards usingISNULLas a replacement #32519)