-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generate OPENJSON with WITH unless ordering is required #30983
Conversation
Converting this back to a draft - there are probably patterns where ordering should be preserved that aren't detected by the current implementation (e.g. projecting out the entire collection as-is, applying an aggregate function that cares about ordering, such as string.Join). Ended up starting with OPENJSON/WITH and an ordering by the (non-existing) key; this is an intermediate representation which is temporarily incorrect. In post-processing, a visitor finds SelectExpressions over OPENJSON where the ordering is still there, and removes the WITH (applying a cast on the projection instead). This leverages the existing logic in the query pipeline to elide ordering (e.g. when applying EXISTS/IN): if the ordering gets elided, we simply maintain the original OPENJSON/WITH; if the ordering is still there in post-processing, we convert it. |
be1f02d
to
8c8cb6e
Compare
src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
@maumar new version up, rebased on latest main. I added two tests per your offline comments:
|
This PR based based on top of #30976 and #30956, review those first and ignore the first commits here.We currently generate SQL Server OPENJSON expressions without WITH, applying a regular relational cast to applying the element store type to the output:
We use this form because it allows preserving the ordering of the JSON array (not needed in the above specific query), and that's not possible with the OPENJSON form that accepts WITH.
This PR switches to using OPENJSON with WITH in contexts where preserving the ordering isn't needed, such as the above query:
Where ordering is important, we continue to use OPENJSON without WITH as today.
Compared to OPENJSON with WITH, OPENJSON without WITH has some performance drawbacks (see #13617 (comment), #13617 (comment)). In addition, some conversions from JSON are only possible when using the WITH variant (see #30727 (comment)).
/cc @yv989c, @pfritschi