Skip to content
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

[release/8.0] Fix OPENJSON postprocessing with split query (#32978) #33027

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

roji
Copy link
Member

@roji roji commented Feb 7, 2024

Fixes #32976, backports #32978

Description

The SqlServerJsonPostprocessor introduced in EF 8 did not properly visit the shaper expression (in addition to the query expression), missing necessary rewriting of split queries which are stored there.

Customer impact

Split queries are an important, commonly-used EF mechanism to avoid severe performance issues (e.g. the so-called "cartesian explosion") when querying out related collections.

JSON postprocessing is a query processing step which is necessary when user queries include a JSON collection that requires ordering (or represents a collection of the SQL Server hierarchyid type). This includes any query which composes an order-sensitive operator on top of a JSON column or parameter, e.g.:

var blogs = await context.Blogs.Where(b => b.Tags[0] == "foo").ToListAsync();

The intersection of these two things - split queries and JSON queries with order sensitivity - currently produce incorrect SQL and fail, and this is what's fixed in this PR. This intersection is likely to be not uncommon.

How found

Customer reported on 8.

Regression

No, as far as we're aware the affected scenarios weren't possible in 8.0.

Testing

Tests added.

Risk

Very low, safe one-line fix; quirk added.

@roji roji requested a review from maumar February 7, 2024 23:57
@roji
Copy link
Member Author

roji commented Feb 8, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@SamMonoRT
Copy link
Member

@roji - is this ready to add the Servicing Consider label for tactics approval?

@roji
Copy link
Member Author

roji commented Feb 29, 2024

@SamMonoRT yes... My bad, it should have been there from the start.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Feb 29, 2024

@roji Please add more background info on why we added SqlServerJsonPostprocessor, what is split query and roughly how likely is it for a user to hit this scenario.

@roji
Copy link
Member Author

roji commented Mar 1, 2024

@AndriySvyryd done, let me know if that seems sufficient.

@maumar maumar merged commit c4fda64 into dotnet:release/8.0 Mar 5, 2024
7 checks passed
@roji roji deleted the VisitShaperInJson8 branch March 5, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants