Support querying over non-primitive JSON collections #31095
Conversation
There was a problem hiding this comment.
One more general comment...
We added element access inside JSON for preview1; this PR adds element access also after other LINQ operators (e.g. Where(x => x.JsonColumn.SomeCollection.Where(y => y > 3).ElementAt(2))). We have two very different paths for translating this: the former works in SharedTypeEntityExpandingExpressionVisitor, the latter uses the generalized TranslateElementAtOrDefault (also used for e.g. primitive collections). This isn't necessary a problem, but we may want to think about a single translation path.
| // FROM (SELECT value ->> 'a' AS a, value ->> 'b' AS b FROM json_each(<JSON column>, <path>)) AS j | ||
| // WHERE j.a = 8; | ||
|
|
||
| // Unfortunately, while the subquery projects the entity, our EntityProjectionExpression currently supports only bare |
There was a problem hiding this comment.
This SQLite implementation was a real PITA... It's hacky but probably "good enough" for now... We should try to make the query pipeline more flexible/powerful to improve this though.
| @@ -448,4 +609,22 @@ private static SqlExpression ApplyTypeMappingOnColumn(SqlExpression expression, | |||
|
|
|||
| _ => expression | |||
| }; | |||
|
|
|||
| private class FakeMemberInfo : MemberInfo | |||
There was a problem hiding this comment.
This is 🤮 🤮 🤮
The problem is that SelectExpression supports either projecting an entity (EntityProjectionExpression), or a single column, or an anonymous type with MemberInfos as the ProjectionMember keys. We should allow ProjectionMember to just be strings.
roji
force-pushed
the
NonPrimitiveJsonCollections
branch
from
3fb1c46 to
847dc09
Compare
roji
force-pushed
the
NonPrimitiveJsonCollections
branch
from
847dc09 to
4aeceab
Compare
|
@maumar freshly rebased for your reviewing pleasure. |
|
|
||
| foreach (var (property, column) in jsonQueryExpression.KeyPropertyMap) | ||
| { | ||
| _identifier.Add((column, property.GetKeyValueComparer())); |
There was a problem hiding this comment.
this is not good enough - key property map only contains the PK of owning entity. It doesn't contain the synthesized "element counter" (since there is nothing to map to), so this is not sufficient identifier.
|
Closing in favor of #31369 which builds on top of this. |
See notes below.
Note that some tests are failing - @maumar if you'd like to take a look. I also think we should add test coverage generally - if you'd like on it that would also be great (though I'd be happy to as well).
Closes #28616