Skip to content

Conversation

@maumar
Copy link
Contributor

@maumar maumar commented Mar 24, 2023

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 requested a review from roji March 24, 2023 00:44
@maumar
Copy link
Contributor Author

maumar commented Mar 24, 2023

@roji broke this and 30266 apart because different risks (in case we want to patch these). So for purpose of this pr just look at the second commit

…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
Copy link
Contributor Author

maumar commented Mar 28, 2023

ping @roji

Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐑 🇮🇹

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants