Skip to content
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: use ReadItem for more Find cases #33896

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Cosmos: use ReadItem for more Find cases #33896

merged 1 commit into from
Jun 5, 2024

Conversation

ajcvickers
Copy link
Contributor

This change stops using ReadItemExpression, since it does not do any shaper processing, and instead uses SelectExpression as for other queries. This allows processing of auto-Includes for owned types, even if Find is being translated to ReadItem.

This is somewhat hacky now, but it likely to change again as the processing for Includes and/or complex types is changed. For now, it's a reasonable way to get an important feature working.

Fixes #24202

In addition, the pattern matching has been updated to detect calls to non-generic Find.

Fixes #33881

@ajcvickers ajcvickers requested a review from a team June 4, 2024 12:03
QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution),
Constant(_threadSafetyChecksEnabled));
var cosmosQueryContextConstant = Convert(QueryCompilationContext.QueryContextParameter, typeof(CosmosQueryContext));
var shaperConstant = Constant(shaperLambda.Compile());
Copy link
Member

Choose a reason for hiding this comment

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

I think we'd need to move out this Compile() to make Cosmos compatible with pre-compiled queries.

@roji De we have a separate issue tracking this work?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, opened #33909 to track Cosmos NativeAOT support.

This change stops using `ReadItemExpression`, since it does not do any shaper processing, and instead uses `SelectExpression` as for other queries. This allows processing of auto-Includes for owned types, even if Find is being translated to `ReadItem`.

This is somewhat hacky now, but it likely to change again as the processing for Includes and/or complex types is changed. For now, it's a reasonable way to get an important feature working.

Fixes #24202

In addition, the pattern matching has been updated to detect calls to non-generic Find.

Fixes #33881
@ajcvickers ajcvickers merged commit 8454b95 into main Jun 5, 2024
7 checks passed
@ajcvickers ajcvickers deleted the RevertToWork branch June 5, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants