-
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
Cosmos: Add embedded collection support #16631
Conversation
67a8e0f
to
ce9357e
Compare
src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Pipeline/CosmosSqlTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
ce9357e
to
fd94c61
Compare
@smitpatel Updated and rebased |
Expression.MakeMemberAccess(entry, _entityTypeMemberInfo))), | ||
MaterializeEntity(entityType, materializationContextVariable, concreteEntityTypeVariable, instanceVariable)))); | ||
Expression.MakeMemberAccess(entryVariable, _entityTypeMemberInfo)), | ||
Expression.Assign(instanceVariable, Expression.Convert( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any significance of different ordering now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consistency. The other blocks end with the instance.
Expression.MakeMemberAccess(entryVariable, _entityMemberInfo), | ||
entityType.ClrType))), | ||
MaterializeEntity( | ||
entityType, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, entryVariable)))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we using entry variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To set IsLoaded
later
return new ProjectionBindingExpression( | ||
_selectExpression, _selectExpression.AddToProjection(translation), expression.Type); | ||
} | ||
//return _selectExpression.AddCollectionProjection( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should remove commented code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we'll need it for non-cross-partition joins
src/EFCore.Cosmos/Query/Pipeline/CosmosProjectionBindingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Pipeline/CosmosProjectionBindingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
fd94c61
to
04b90b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
04b90b5
to
7d4b425
Compare
Flatten out shaper expression visitors
Don't expand owned collection
Fixes #16620
Part of #12086