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

Query/Json: projecting entity collection along with json collection generates invalid shaper #30565

Closed
maumar opened this issue Mar 23, 2023 · 1 comment · Fixed by #30568
Closed
Labels
area-json area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Mar 23, 2023

Similar to #30266 but for json collections, separated because fix is slightly more involved/disruptive. When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed.

maumar added a commit that referenced this issue Mar 24, 2023
…son collection generates invalid shaper

When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed. Basically, we would generate code for collection projection outside of `resultContext.Values == null` block (this needs to happen for regular collections due to result coordination). For JSON it's not needed, because entire object is loaded from a single row. So we can move the processing into the block and then just refer to it's product in the final projection. We also need to shuffle around the order in which we build the block. We should build expressions and json entities first, then populate resultContext and then build includes (which depend on values in the context). Before we were populating result values before we generated json entities, (which didn't matter because we were not using that info) but now is necessary.

Fixes #30565
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 24, 2023
@ajcvickers ajcvickers added this to the 8.0.0 milestone Mar 24, 2023
maumar added a commit that referenced this issue Mar 25, 2023
…son collection generates invalid shaper

When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed. Basically, we would generate code for collection projection outside of `resultContext.Values == null` block (this needs to happen for regular collections due to result coordination). For JSON it's not needed, because entire object is loaded from a single row. So we can move the processing into the block and then just refer to it's product in the final projection. We also need to shuffle around the order in which we build the block. We should build expressions and json entities first, then populate resultContext and then build includes (which depend on values in the context). Before we were populating result values before we generated json entities, (which didn't matter because we were not using that info) but now is necessary.

Fixes #30565
maumar added a commit that referenced this issue Mar 29, 2023
…son collection generates invalid shaper (#30568)

When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed. Basically, we would generate code for collection projection outside of `resultContext.Values == null` block (this needs to happen for regular collections due to result coordination). For JSON it's not needed, because entire object is loaded from a single row. So we can move the processing into the block and then just refer to it's product in the final projection. We also need to shuffle around the order in which we build the block. We should build expressions and json entities first, then populate resultContext and then build includes (which depend on values in the context). Before we were populating result values before we generated json entities, (which didn't matter because we were not using that info) but now is necessary.

Fixes #30565
@maumar
Copy link
Contributor Author

maumar commented Mar 29, 2023

reopening for patch

@maumar maumar reopened this Mar 29, 2023
@maumar maumar modified the milestones: 8.0.0, 7.0.x Mar 29, 2023
maumar added a commit that referenced this issue Mar 30, 2023
…as regular entity collection

#30266

Problem was that when we projection collection of entities we use result coordinator and generate different pattern in shaper code. Json entity shaper code is generated inside `resultContext.Values == null` block and all the products should be referred to via resultContext.Values array access.
We were not doing that for json, so in the final projection code we would refer to parameters (storing the actual json entities) that were out of scope.
Fix is to use the correct result coordinator pattern, just like we do for non-json entities.

#30565

When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed. Basically, we would generate code for collection projection outside of `resultContext.Values == null` block (this needs to happen for regular collections due to result coordination). For JSON it's not needed, because entire object is loaded from a single row. So we can move the processing into the block and then just refer to it's product in the final projection. We also need to shuffle around the order in which we build the block. We should build expressions and json entities first, then populate resultContext and then build includes (which depend on values in the context). Before we were populating result values before we generated json entities, (which didn't matter because we were not using that info) but now is necessary.

Fixes #30266
Fixes #30565
maumar added a commit that referenced this issue Apr 3, 2023
…as regular entity collection

#30266

Problem was that when we projection collection of entities we use result coordinator and generate different pattern in shaper code. Json entity shaper code is generated inside `resultContext.Values == null` block and all the products should be referred to via resultContext.Values array access.
We were not doing that for json, so in the final projection code we would refer to parameters (storing the actual json entities) that were out of scope.
Fix is to use the correct result coordinator pattern, just like we do for non-json entities.

#30565

When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed. Basically, we would generate code for collection projection outside of `resultContext.Values == null` block (this needs to happen for regular collections due to result coordination). For JSON it's not needed, because entire object is loaded from a single row. So we can move the processing into the block and then just refer to it's product in the final projection. We also need to shuffle around the order in which we build the block. We should build expressions and json entities first, then populate resultContext and then build includes (which depend on values in the context). Before we were populating result values before we generated json entities, (which didn't matter because we were not using that info) but now is necessary.

Fixes #30266
Fixes #30565
ajcvickers pushed a commit that referenced this issue Apr 5, 2023
…rence as well as regular entity collection (#30600)

Fixes #30266
Fixes #30565
@ajcvickers ajcvickers modified the milestones: 7.0.x, 7.0.6 Apr 5, 2023
@ajcvickers ajcvickers modified the milestones: 7.0.6, 7.0.7 Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-json area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
2 participants