Cosmos: Implement SelectMany #34013
Cosmos: Implement SelectMany #34013
Conversation
roji
force-pushed
the
CosmosSelectMany
branch
2 times, most recently
from
112de70
to
8bb7df1
Compare
| @@ -123,6 +123,9 @@ | |||
| <data name="CanConnectNotSupported" xml:space="preserve"> | |||
| <value>The Cosmos database does not support 'CanConnect' or 'CanConnectAsync'.</value> | |||
| </data> | |||
| <data name="ComplexProjectionInSubqueryNotSupported" xml:space="preserve"> | |||
| <value>Complex projections in subqueries are currently unsupported.</value> | |||
There was a problem hiding this comment.
A second sentence on what they can do about this?
There was a problem hiding this comment.
Unfortunately I'm not sure what can be done :(
I think we should try to fix this for 9.0 - that's #34004, and I think it actually shouldn't be too hard. But it does involve some shaper changes...
Let's discuss this (but I'll go ahead and merge for now).
| { | ||
| _sqlBuilder.Append("VALUE "); | ||
|
|
||
| if (projection is not [var singleProjection]) | ||
| { | ||
| throw new UnreachableException( |
There was a problem hiding this comment.
Let's discuss UnreachableExceptions in a design meeting. :-)
| // .SelectMany(p => Property(p, "Ints").AsQueryable(), (p, c) => new TransparentIdentifier`2(Outer = p, Inner = c)) | ||
| // .Select(ti => ti.Inner) | ||
|
|
||
| // TODO: In Cosmos, we currently always add a predicate for the discriminator, so the source is never a bare array. |
There was a problem hiding this comment.
You can always use HasNoDiscriminator in tests.
Meta-point: too many of our tests have:
- Discriminators by default
- No partition key
- No owned types
I think this means we're largely testing un-representative models.
There was a problem hiding this comment.
Good point, with HasNoDiscriminator() there's indeed no predicate. But unfortunately there's still the problem described just above, where nav expansion rewrites the simple SelectMany() to the complete form with a result selector, and an additional Select that projects the inner out. So doing the simplification here to remove the JOIN is pretty much unfeasible until we fix that...
There was a problem hiding this comment.
Meta-point: too many of our tests have [...]
Yeah, you're right. For the discriminators, we've been recently discussing changing our default mapping strategy, i.e. no longer mapping different entity types to the same container by default; if go decide to do that, presumably there won't be a discriminator by default in any case (i.e. Cosmos will be more similar to relational), so at least that point maybe goes away.
| // the JOIN source. | ||
| // However, the JSON object being projected out of the subquery doesn't correspond to any entity type, and there's currently | ||
| // no way for us to represent a reference to that - ObjectReferenceExpression requires an IEntityType. Changing that | ||
| // requires shaper-side changes (see comment in ObjectReferenceExpression); if we can remove that requirement, we can |
Also introduces substantial infrastructure for general joins Closes dotnet#17312
NOTE: This PR is based on top of #33998, skip first two commits
Also introduces substantial infrastructure for general joins; actual Join operator support to be introduced shortly.
Closes #17312