-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Json reader sandbox rebase #31076
Json reader sandbox rebase #31076
Conversation
@ajcvickers fyi |
edc51f8
to
e0b0e8c
Compare
@ajcvickers pushed new version, updated to use your new json type mapping infra and did some clean up |
@@ -318,6 +318,8 @@ protected override Expression VisitShapedQuery(ShapedQueryExpression shapedQuery | |||
shapedQueryExpression.ShaperExpression, | |||
out var relationalCommandCache, out var readerColumns, out var relatedDataLoaders, ref collectionCount); | |||
|
|||
var foo = ExpressionPrinter.Print(shaper); |
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.
breakpoint here to see the see the final expression for debugging
} | ||
|
||
shaperBlockExpressions.Add(entityShaperMaterializerVariable); | ||
var rewrittenEntityShaperMaterializer = new JsonEntityMaterializerRewriter2( |
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.
breakpoint here to see the shaper before (i.e. regular entity materializer with random access to the properties in the reader/JSON DOM) and after (basic streaming structure, but without JSON property reads yet - still all in terms of ValueBuffers). Still doesn't handle the tracking scenario (where we already have entity in state manager but need to read/skip through json stream rather than return instantly.
{ | ||
var (jsonElementParameter, keyPropertyValuesParameter) = _jsonMaterializationContextParameterMapping[mappingParameter]; | ||
var (jsonReaderDataParameter, keyPropertyValuesParameter) = _jsonMaterializationContextToJsonReaderDataAndKeyValuesParameterMapping[mappingParameter]; |
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.
here is where we actually convert from ValueBuffer based dummy property access to actual JsonReaderWriter stuff (inside CreateReadJsonPropertyValueExpression). Seems to be working fine, but idk about AOT
e0b0e8c
to
9a672b6
Compare
very hacky, but most tests (apart from deduplication) work