Fix: Cosmos ToList on structural collection is treated as scalar#38122
Fix: Cosmos ToList on structural collection is treated as scalar#38122JoasE wants to merge 1 commit intodotnet:mainfrom
Conversation
… projection Don't bind non scalar subquery results for ToList projections
JoasE
changed the title
There was a problem hiding this comment.
I think these changes should be temporary, and #34067 will revert them.
I'm not as familiar with the projection binding code as other area's, but FWIG, this is the best solution right now
A bit of extra explanation per test case:
| """ | ||
| SELECT VALUE o["Details"] | ||
| SELECT VALUE o | ||
| FROM root c |
There was a problem hiding this comment.
This projection now happens on the client side in the materializer. This used to generate a JsonConvert.Deserialize call for materialization
There was a problem hiding this comment.
Because this is a SelectMany, we do bind the c["Orders"], but the "Details" projection part happens in the materializer
| public override Task Json_collection_leaf_filter_in_projection(bool async) | ||
| => Fixture.NoSyncTest( | ||
| async, async a => | ||
| { |
There was a problem hiding this comment.
This used to generate a JsonConvert.Deserialize call for materialization. I think we can't translate this because the Where would have to be ran in the materializer since we don't bind ["OwnedReferenceRoot"]["OwnedReferenceBranch"]["OwnedCollectionLeaf"], and 34067 should solve that
| public override Task Distinct() | ||
| => AssertTranslationFailed(base.Distinct); | ||
|
|
||
| public override async Task Distinct_projected(QueryTrackingBehavior queryTrackingBehavior) |
There was a problem hiding this comment.
This used to generate a JsonConvert.Deserialize call for materialization. I think we can't translate this because the Distinct would have to be ran in the materializer since we don't bind ["AssociateCollection"], and 34067 should solve that
JoasE
changed the title
JoasE
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes Cosmos query projection binding for ToList() over structural (non-scalar) collections by avoiding scalar projection binding for non-scalar subquery results.
Changes:
- Adjust Cosmos projection binding so
ToList()only binds scalar array subquery results as a scalar projection; non-scalar results are visited/shaped instead. - Update Cosmos functional test SQL baselines for owned structural collection
ToList()projections. - Update/realign Cosmos tests to assert translation failure for scenarios that were previously (incorrectly) translated for structural collections (e.g.
Where/Distinctcomposed beforeToList()).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs | Updates expected Cosmos SQL to project the owning object (o) rather than o["Details"] for structural collection ToList() projections. |
| test/EFCore.Cosmos.FunctionalTests/Query/JsonQueryCosmosTest.cs | Changes a JSON structural collection filter-in-projection test to assert translation failure instead of asserting generated SQL. |
| test/EFCore.Cosmos.FunctionalTests/Query/Associations/ComplexProperties/ComplexPropertiesCollectionCosmosTest.cs | Changes Distinct_projected to assert translation failure rather than asserting generated SQL. |
| src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs | Prevents binding non-scalar array subquery results (e.g. structural/object arrays) as scalar projections when translating ToList(); adds a translation-failure throw for unexpected Queryable methods in this client-eval path. |
Don't bind non scalar subquery results for ToList projections
This would change behaviour from 10.0.0, where some queries compiled and might have worked, but actually didn't use a real materializer. If the user didn't have any custom ef config and their model matches the json
Closes: #38121