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

Improvement to Find/FindAsync with Partition Key #20461

Merged
merged 3 commits into from
Apr 19, 2020
Merged

Improvement to Find/FindAsync with Partition Key #20461

merged 3 commits into from
Apr 19, 2020

Conversation

1iveowl
Copy link
Contributor

@1iveowl 1iveowl commented Mar 29, 2020

Suggested improvements for Find/FindAsync for Cosmos, allowing for ReadItem optimization, when the Partition Key is known and Cosmos Resource Id is provided or Cosmos Resource Id can be generated based on the default generator or a custom generator.

Please see #17310

Copy link
Contributor Author

@1iveowl 1iveowl left a comment

Choose a reason for hiding this comment

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

Comment and questions to changes.

Copy link
Member

@smitpatel smitpatel left a comment

Choose a reason for hiding this comment

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

The fix should not change ParameterExtractingExpressionVisitor in anyway. That is not what it meant for. Wait for @AndriySvyryd to provide his thoughts on how to fix.

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 30, 2020

The fix should not change ParameterExtractingExpressionVisitor in anyway. That is not what it meant for. Wait for @AndriySvyryd to provide his thoughts on how to fix.

Thank you @smitpatel. Changing the approach shouldn't be a big issue. The hard part was not writing the code, rather it was figuring out how it all works together in the Cosmos provider and then pick the best approach for implementing the suggested improvements, in a manner that best aligns with the existing pattern etc., and with minimal changes.

Regarding ParameterExtractingExpressionVisitor, please let me share my thinking behind why I picked this as my focus point. Understanding some of my thinking might make it easier for you to guide me to where you'd rather want me to go.

Technically I haven't changed ParameterExtractingExpressionVisitor, I've extended it. I did this because I didn't want to rewrite the logic of ParameterExtractingExpressionVisitor. I considered creating an all new visitor for traversing query and extracting the parameters I need for the ReadItem optimization, but when I at ParameterExtractingExpressionVisitor I reasoned that writing a new visitor seemed to create a great deal of overlap with what is already there. So, by extending ParameterExtractingExpressionVisitor, I sought to extract the additional parameters, without disrupting the existing functionality and dependencies.

Anyhow, I'm sure there are other ways to collect those needed parameters. Would be great if your and/or @AndriySvyryd could point me in the direction of the approach you'd want me to take.

@smitpatel
Copy link
Member

I'm sure there are other ways to collect those needed parameters.

What are the parameters you need to collect?

@ajcvickers
Copy link
Member

@1iveowl

The hard part was not writing the code, rather it was figuring out how it all works together in the Cosmos provider and then pick the best approach for implementing the suggested improvements, in a manner that best aligns with the existing pattern etc., and with minimal changes.

Believe me, this is how I feel whenever I look at the query code! 😸 Thank you for your efforts here!

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 30, 2020

I'm sure there are other ways to collect those needed parameters.

What are the parameters you need to collect?

@smitpatel what I need to extract from the query is:

  • The partition key: property name and value.
  • The property name and value pairs of all the primary keys.
  • The resource id value, if it is provided.

It's all collected by CosmosQueryContext, which inherits from QueryContext and implements the interface ICosmosParameterValues

Seems easy enough, right 😊

I also look to see if the query is attempting to fetch only one item - i.e if the ShapedQueryExpression has ResultCardinality.Single or ResultCardinality.SingleOrDefault. See here.

It all comes together here in these few lines of code, where it is decided if the Cosmos query request can be fulfilled using the new ReadItem optimization or if the original FeedIterator should be used. The result of this decision is expressed by the value of the _isSingleItemQuery boolean.

@smitpatel
Copy link
Member

So assumptions about first EntityType, setting EntityType/PK is all incorrect thing to put on QueryContext. If you see in translation phase, you will have a query root expression which will tell you which IEntityType it is. From IEntityType you can get IProperties which makes PK. From IEntityType you can also fetch IProperty for "id" and it's value generator. So from query perspective, whole processing should be around compilation time, no changes should be made to ParameterExtractingEV.
The best approach would be look at the expression tree you get inside QueryTranslationPreprocessor for Find query and see how you can identify that pattern and provide an alternate translation for it.
Also from CosmosClientWrapper side, it would be good to keep the method which gets one item using ReadItem separate from FeedIterator. You could using QueryingEnumerable from query side but perhaps generate a different enumerable.

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 30, 2020

If you see in translation phase, you will have a query root expression which will tell you which IEntityType it is

What about model.GetEntityTypes().First() is this a no-no for getting the IEntityType?

@smitpatel
Copy link
Member

It's a big no-no.

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 30, 2020

The best approach would be look at the expression tree you get inside QueryTranslationPreprocessor

I.e. using the CosmosQueryTranslationPreprocessor and the CosmosQueryMetadataExtractingExpressionVisitor that was created for the WithPartitionKey(..) extension?

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 30, 2020

If you see in translation phase, you will have a query root expression which will tell you which IEntityType it is

This query passed to the NormalizeQueryableMethodCall?

@smitpatel
Copy link
Member

Yes, in that method you can inspect the expression tree. (Use new ExpressionPrinter().Print(query) to get a good debug output). From we can figure out how do we reach the final query which we want to. Can you also paste debug output here?

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 30, 2020

Yes, in that method you can inspect the expression tree. (Use new ExpressionPrinter().Print(query) to get a good debug output). From we can figure out how do we reach the final query which we want to. Can you also paste debug output here?

Here's the print:

DbSet<Customer_WithResourceId>()
.FirstOrDefault(e => EF.Property(e, "PartitionKey") == __p_0 && EF.Property(e, "id") == __p_1 && EF.Property(e, "Id") == __p_2)

Here's the debugview of the query:

.Call System.Linq.Queryable.FirstOrDefault(
.Extension<Microsoft.EntityFrameworkCore.Query.QueryRootExpression>,
'(.Lambda #Lambda1<System.Func`2[Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId,System.Boolean]>))

.Lambda #Lambda1<System.Func`2[Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId,System.Boolean]>(Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId $e)
{
.Call Microsoft.EntityFrameworkCore.EF.Property(
$e,
"PartitionKey") == $__p_0 && .Call Microsoft.EntityFrameworkCore.EF.Property(
$e,
"id") == $__p_1 && .Call Microsoft.EntityFrameworkCore.EF.Property(
$e,
"Id") == $__p_2
}

How do I extract the IEntityType from the query?

@smitpatel
Copy link
Member

Microsoft.EntityFrameworkCore.Query.QueryRootExpression has EntityType property.

@smitpatel
Copy link
Member

In first pass, let's just optimize queries coming from Find where PK properties contain id & partition key both.
Step 1: Identify the query shape and skip the translation pipeline.
Override CosmosQueryableMethodTranslatingExpressionVisitor.Visit method. You will get same query expression as above. You pattern match it with following

  • MethodCallExpression
  • Method is Queryable.FirstOrDefault
  • Source is QueryRootExpression - get entityType from QueryRootExpression
  • predicate is non-null
  • Predicate is binary expression with ExpressionType.AndAlso where binaryExpression.Left is ExpressionType.Equal and binaryExpression.Right is either ExpressionType.AndAlso with recursive structure or Expression.Equal.
  • Collect all equalty comparisons from predicate, Left side of all comparison should EF.Property method call.
  • Use EntityType from QueryRootExpression and use propertyname from EF.Property method calls (use TryGetEFPropertyArguments method), get set of IProperty. Ordering of equality comparisons is important.
  • Once you get list of IProperties, match it with EntityType.FindPrimaryKey().Properties.

If all above matches, then you have a Find query. We can relax the pattern bit more in future so user written query which are similar to that can also be optimized, for example doing First/Single/Last or default with key values all could be optimized.

Now if the pattern is identified then you create a new ShapedQueryExpression like below

            return new ShapedQueryExpression(
                selectExpression,
                new EntityShaperExpression(
                    entityType,
                    new ProjectionBindingExpression(
                        selectExpression,
                        new ProjectionMember(),
                        typeof(ValueBuffer)),
                    false));

But instead of using selectExpression, you need to add another custom expression ReadItemExpression. ReadItemExpression should contain properties/field for things you need to remember to run ReadItemAsync on Cosmos.
From comparisons above, you will have parameter values for "id" & Partition key. you store them on ReadItemExpression. And you return shaped query created above directly skipping whole translation pipeline. Also set ShapedQueryExpression's ResultCardinality here.

  • Cosmos does not have QueryTranslationPostprocessor so no changes needed there.

Step 2: Taking care of materialization
In method CosmosShapedQueryCompilingExpressionVisitor.VisitShapedQueryExpression,
You can check if shapedQueryExpression.QueryExpression is SelectExpression or ReadItemExpression.
if it is ReadItemExpression then you need to skip processing does for SelectExpression.
Hence

  • ApplyProjection call is not needed.
  • You would need to inject JObject & EntityMaterializers.
  • The call to CosmosProjectionBindingRemovingExpressionVisitor.Visit is the case where you would need to write a different visitor which would bind the entityMaterializer to object being returned from ReadItemAsync method call. You can inspect expression tree at this point. EntityMaterializer takes a value buffer and generates entity instance. You would need to replace the calls to each value buffer read from particular index to the JObject you get from server side and property read. You can some inspiration from CosmosProjectionBindingRemovingExpressionVisitor. Especially you will need CreateGetValueExpression from that class.

Step 3 execute the query.

  • Given the arguments for QueryingEnumerable, I would suggest using a different enumerable to be used with ReadItemAsync.
  • You will get values of parameters from QueryContext.ParameterValues, get values from there and pass to ReadItemAsync.
  • For ReadItemAsync, create a new method in CosmosClientWrapper which does that and being called from above queryingEnumerable you created.

We can expand a lot more in future. This is minimum amount of basic stuff we need to add in first pass.
Let me know if you have questions or get stuck anywhere. Expression trees are hard 😉

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 31, 2020

  • Predicate is binary expression with ExpressionType.AndAlso where binaryExpression.Left is ExpressionType.Equal and binaryExpression.Right is either ExpressionType.AndAlso with recursive structure or Expression.Equal.

[Update]

public virtual Expression CreateShapedReadItemQueryExpression(Expression expression)
{
    if (methodCallExpression.Method.IsGenericMethod
        && methodCallExpression.Method.Name == nameof(Queryable.FirstOrDefault))
    {
        if (methodCallExpression.Arguments[0] is MethodCallExpression queryRootMethodCallExpression
            && methodCallExpression.Method.IsGenericMethod
            && queryRootMethodCallExpression.Method.Name == nameof(Queryable.Where))
        {
            if (queryRootMethodCallExpression.Arguments[0] is QueryRootExpression queryRootExpression)
            {
                // Store EntityType in ReadItemExpression;
            }

            if (queryRootMethodCallExpression.Arguments[1] is UnaryExpression unaryExpression
                && unaryExpression.Operand is LambdaExpression lambdaExpression
                && lambdaExpression.Body is BinaryExpression binaryExpression)
            {
                VisitBinary(binaryExpression);
            }
        }
    }
}

Overriding VisitBinary for the extraction of the property names and extraction of the expression for getting their values.

@smitpatel
Copy link
Member

You would need recursive method to identify structure of predicate since, it would have more nesting as number of PK properties increases. We can defer it for later.

@smitpatel
Copy link
Member

Also don't set _find and call base visitor. Short-circuit entire thing.
Create a new ShapedQueryExpression as I described above.

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 31, 2020

You would need recursive method to identify structure of predicate since, it would have more nesting as number of PK properties increases. We can defer it for later.

Thank you. The recursive part makes better sense to me. I'd misunderstood the guide on this part and I had a misunderstanding on how the ExpressionVisitor works. I thought that it would traverse the tree by itself. It makes much more sense with the recursive call to VisitBinary.

I've updated the code in the comment above to reflect this.

@smitpatel
Copy link
Member

It would be actually separate method. Rather than base call. We cannot use ExpressionVisitor for that.
Have a look at this code which essentially does the same, (though it is right leaning tree so you may have to flip a little)

private static (Expression, Expression) DecomposeJoinCondition(Expression joinCondition)
{
var leftExpressions = new List<Expression>();
var rightExpressions = new List<Expression>();
return ProcessJoinCondition(joinCondition, leftExpressions, rightExpressions)
? leftExpressions.Count == 1
? (leftExpressions[0], rightExpressions[0])
: (CreateAnonymousObject(leftExpressions), CreateAnonymousObject(rightExpressions))
: (null, null);
static Expression CreateAnonymousObject(List<Expression> expressions)
=> Expression.New(
AnonymousObject.AnonymousObjectCtor,
Expression.NewArrayInit(
typeof(object),
expressions.Select(e => Expression.Convert(e, typeof(object)))));
}
private static bool ProcessJoinCondition(
Expression joinCondition, List<Expression> leftExpressions, List<Expression> rightExpressions)
{
if (joinCondition is BinaryExpression binaryExpression)
{
if (binaryExpression.NodeType == ExpressionType.Equal)
{
leftExpressions.Add(binaryExpression.Left);
rightExpressions.Add(binaryExpression.Right);
return true;
}
if (binaryExpression.NodeType == ExpressionType.AndAlso)
{
return ProcessJoinCondition(binaryExpression.Left, leftExpressions, rightExpressions)
&& ProcessJoinCondition(binaryExpression.Right, leftExpressions, rightExpressions);
}
}
return false;
}

It collects Expressions from left & right. You will get EF.Property calls on left & ParameterExpression on right.

@1iveowl
Copy link
Contributor Author

1iveowl commented Mar 31, 2020

  • ... get set of IProperty. Ordering of equality comparisons is important.. Ordering of equality comparisons is important.

@smitpatel Can you elaboate here. By this do you mean that the property names should be matched by index, rather than merely their names? Or?

@smitpatel
Copy link
Member

If predicate is
EF.Property(a, "id") == p0 && EF.Property(a, "PartitionKey") == p1)`
then you need to also remember the order of "id" and "PartitionKey" so when you match them with PrimaryKey.Properties, they appear in exact same order.

@1iveowl
Copy link
Contributor Author

1iveowl commented Apr 3, 2020

@smitpatel I'm a bit stuck. Any hints on the following?

When looking at the return value from [VisitShapedQueryExpression] in DebugView:

var serverEnumerable = VisitShapedQueryExpression(shapedQueryExpression);

...I'm getting this exception, but only in the ReadItemExpression code path:

Extension node must override the property Expression.NodeType.

The new VisitShapedQueryExpression looks like this.

protected override Expression VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression)
{
    Check.NotNull(shapedQueryExpression, nameof(shapedQueryExpression));

    var jObjectParameter = Expression.Parameter(typeof(JObject), "jObject");

    var shaperBody = shapedQueryExpression.ShaperExpression;
    shaperBody = new JObjectInjectingExpressionVisitor().Visit(shaperBody);
    shaperBody = InjectEntityMaterializers(shaperBody);

    switch (shapedQueryExpression.QueryExpression)
    {
        case SelectExpression selectExpression:
            selectExpression.ApplyProjection();

            shaperBody = new CosmosProjectionBindingRemovingExpressionVisitor(selectExpression, jObjectParameter, IsTracking)
                .Visit(shaperBody);

            var shaperLambda = Expression.Lambda(
                shaperBody,
                QueryCompilationContext.QueryContextParameter,
                jObjectParameter);

            return Expression.New(
                typeof(QueryingEnumerable<>).MakeGenericType(shaperLambda.ReturnType).GetConstructors()[0],
                Expression.Convert(
                    QueryCompilationContext.QueryContextParameter,
                    typeof(QueryContext)),
                Expression.Constant(_sqlExpressionFactory),
                Expression.Constant(_querySqlGeneratorFactory),
                Expression.Constant(selectExpression),
                Expression.Constant(shaperLambda.Compile()),
                Expression.Constant(_contextType),
                Expression.Constant(_partitionKeyFromExtension, typeof(string)),
                Expression.Constant(_logger));

        case ReadItemExpression readItemExpression:

            shaperBody =
                new CosmosProjectionBindingRemovingReadItemExpressionVisitor(readItemExpression, jObjectParameter, IsTracking)
                .Visit(shaperBody);

            var shaperReadItemLambda = Expression.Lambda(
                shaperBody,
                QueryCompilationContext.QueryContextParameter,
                jObjectParameter);

            return Expression.New(
                typeof(ReadItemQueryingEnumerable<>).MakeGenericType(shaperReadItemLambda.ReturnType).GetConstructors()[0],
                Expression.Convert(
                    QueryCompilationContext.QueryContextParameter,
                    typeof(QueryContext)),
                Expression.Constant(readItemExpression),
                Expression.Constant(shaperReadItemLambda.Compile()),
                Expression.Constant(_contextType),
                Expression.Constant(_logger));
            
        default:
            return null;
    }
}

The query expression that goes into return Expression.New(typeof(ReadItemQueryingEnumerable<>)...) looks like the following:

.Lambda #Lambda1<System.Func`3[Microsoft.EntityFrameworkCore.Query.QueryContext,Newtonsoft.Json.Linq.JObject,Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId]>(
    Microsoft.EntityFrameworkCore.Query.QueryContext $queryContext,
    Newtonsoft.Json.Linq.JObject $jObject) {
    .Block(Newtonsoft.Json.Linq.JObject $jObject1) {
        $jObject1 = .Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject.get_Item("c")
        );
        .If ($jObject1 == null) {
            null
        } .Else {
            .Block(
                Microsoft.EntityFrameworkCore.Storage.MaterializationContext $materializationContext1,
                Microsoft.EntityFrameworkCore.Metadata.IEntityType $entityType1,
                Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId $instance1,
                Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry $entry1,
                System.Boolean $hasNullKey1) {
                $materializationContext1 = .New Microsoft.EntityFrameworkCore.Storage.MaterializationContext(
                    .Constant<Microsoft.EntityFrameworkCore.Storage.ValueBuffer>(Microsoft.EntityFrameworkCore.Storage.ValueBuffer),
                    $queryContext.Context);
                $instance1 = null;
                $entry1 = .Call ($queryContext.StateManager).TryGetEntry(
                    .Constant<Microsoft.EntityFrameworkCore.Metadata.Internal.Key>(Key: Customer_WithResourceId.PartitionKey, Customer_WithResourceId.id, Customer_WithResourceId.Id PK),
                    .NewArray System.Object[] {
                        .Invoke (.Lambda #Lambda2<System.Func`2[Newtonsoft.Json.Linq.JToken,System.Object]>)(.Call $jObject1.get_Item("PartitionKey")
                        ),
                        (System.Object).Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("id")
                        ),
                        (System.Object).Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("Id")
                        )
                    },
                    True,
                    $hasNullKey1);
                .If (
                    !$hasNullKey1
                ) {
                    .If ($entry1 != null) {
                        .Block() {
                            $entityType1 = $entry1.EntityType;
                            $instance1 = (Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId)$entry1.Entity
                        }
                    } .Else {
                        .Block(Microsoft.EntityFrameworkCore.Storage.ValueBuffer $shadowValueBuffer1) {
                            $shadowValueBuffer1 = .Constant<Microsoft.EntityFrameworkCore.Storage.ValueBuffer>(Microsoft.EntityFrameworkCore.Storage.ValueBuffer);
                            $entityType1 = .Block(System.String $discriminator) {
                                $discriminator = .Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("Discriminator")
                                );
                                .Switch ($discriminator) {
                                .Case ("Customer_WithResourceId"):
                                        .Constant<Microsoft.EntityFrameworkCore.Metadata.IEntityType>(EntityType: Customer_WithResourceId)
                                .Default:
                                        .Block() {
                                            .Throw .Call Microsoft.EntityFrameworkCore.Query.EntityShaperExpression.CreateUnableToDiscriminateException(
                                                .Constant<Microsoft.EntityFrameworkCore.Metadata.Internal.EntityType>(EntityType: Customer_WithResourceId),
                                                (System.Object)$discriminator);
                                            null
                                        }
                                }
                            };
                            $instance1 = .Switch ($entityType1) {
                            .Case (.Constant<Microsoft.EntityFrameworkCore.Metadata.IEntityType>(EntityType: Customer_WithResourceId)):
                                    .Block() {
                                        $shadowValueBuffer1 = .New Microsoft.EntityFrameworkCore.Storage.ValueBuffer(.NewArray System.Object[] {
                                                (System.Object).Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("Discriminator")
                                                ),
                                                $jObject1
                                            });
                                        .Block(Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId $instance) {
                                            $instance = .New Microsoft.EntityFrameworkCore.Cosmos.EndToEndCosmosTest+Customer_WithResourceId();
                                            $instance.<PartitionKey>k__BackingField = .Invoke (.Lambda #Lambda3<System.Func`2[Newtonsoft.Json.Linq.JToken,System.Int32]>)(.Call $jObject1.get_Item("PartitionKey")
                                            );
                                            $instance.<id>k__BackingField = .Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("id")
                                            );
                                            $instance.<Id>k__BackingField = (System.Int32).Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("Id")
                                            );
                                            $instance.<Name>k__BackingField = .Call Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.CosmosShapedQueryCompilingExpressionVisitor+CosmosProjectionBindingRemovingReadItemExpressionVisitor.SafeToObject(.Call $jObject1.get_Item("Name")
                                            );
                                            $instance
                                        }
                                    }
                            .Default:
                                    null
                            };
                            $entry1 = .Call $queryContext.StartTracking(
                                $entityType1,
                                $instance1,
                                $shadowValueBuffer1);
                            $instance1
                        }
                    }
                } .Else {
                    .Default(System.Void)
                };
                $instance1
            }
        }
    }
}

.Lambda #Lambda2<System.Func`2[Newtonsoft.Json.Linq.JToken,System.Object]>(Newtonsoft.Json.Linq.JToken $var1) {
    .If (
        $var1 == .Default(Newtonsoft.Json.Linq.JToken) || $var1.Type == .Constant<Newtonsoft.Json.Linq.JTokenType>(Null)
    ) {
        .Default(System.Object)
    } .Else {
        (System.Object).Block(System.Int32 $parsed) {
            .If (
                .Call System.Int32.TryParse(
                    .Call $var1.ToObject(),
                    .Constant<System.Globalization.NumberStyles>(Any),
                    .Constant<System.IFormatProvider>(),
                    $parsed)
            ) {
                $parsed
            } .Else {
                0
            }
        }
    }
}

.Lambda #Lambda3<System.Func`2[Newtonsoft.Json.Linq.JToken,System.Int32]>(Newtonsoft.Json.Linq.JToken $var2) {
    .If (
        $var2 == .Default(Newtonsoft.Json.Linq.JToken) || $var2.Type == .Constant<Newtonsoft.Json.Linq.JTokenType>(Null)
    ) {
        .Default(System.Int32)
    } .Else {
        .Block(System.Int32 $parsed) {
            .If (
                .Call System.Int32.TryParse(
                    .Call $var2.ToObject(),
                    .Constant<System.Globalization.NumberStyles>(Any),
                    .Constant<System.IFormatProvider>(),
                    $parsed)
            ) {
                $parsed
            } .Else {
                0
            }
        }
    }
}

@smitpatel
Copy link
Member

Extension node must override the property Expression.NodeType.

Make sure that you override Type & NodeType property in ReadItemExpression class.

@1iveowl 1iveowl requested a review from smitpatel April 4, 2020 11:38
Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

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

Looks good. Please squash and rebase on latest master

@1iveowl
Copy link
Contributor Author

1iveowl commented Apr 19, 2020

Looks good. Please squash and rebase on latest master

When rebasing to latest master, I can no longer open the solution. Not sure what's going on. All projects end up unloaded.

@AndriySvyryd
Copy link
Member

When rebasing to latest master, I can no longer open the solution. Not sure what's going on. All projects end up unloaded.

Run build.cmd then open the solution using startvs.cmd

@1iveowl
Copy link
Contributor Author

1iveowl commented Apr 19, 2020

When rebasing to latest master, I can no longer open the solution. Not sure what's going on. All projects end up unloaded.

Run build.cmd then open the solution using startvs.cmd

Ahhh, the framework got updated.

@1iveowl 1iveowl closed this Apr 19, 2020
@1iveowl
Copy link
Contributor Author

1iveowl commented Apr 19, 2020

I see you've added netcoreapp5.0 tests now too. I assume this means that the tests I've written and/or changed for 3.1 need to be updated for 5.0 too?

I can open the solution with startvs.cmd now. I get a number of build errors, like this now:

Microsoft.EntityFrameworkCore.ChangeTracking.EntityEntry is an internal API that supports the Entity Framework Core infrastructure and not subject to the same compatibility standards as public APIs. It may be changed or removed without notice in any release.

I will continue looking at it tomorrow.

@1iveowl 1iveowl reopened this Apr 19, 2020
@AndriySvyryd
Copy link
Member

Suppress that warning in the source code

@1iveowl
Copy link
Contributor Author

1iveowl commented Apr 19, 2020

Suppress that warning in the source code

Done.

#pragma warning disable EF1001 added where needed.

@1iveowl
Copy link
Contributor Author

1iveowl commented Apr 19, 2020

Regarding squashing the commits, I cannot get rid of the 3 rebase to master I did along the way, so unable to squash below 3 commits.

Probably due to my git skills coming to a short here.

author Jasper Hedegaard Bojsen <jasperhb@outlook.com> 1584698113 +0100
committer Jasper Hedegaard Bojsen <jasperhb@outlook.com> 1586933746 +0200

parent 2972c62
author Jasper Hedegaard Bojsen <jasperhb@outlook.com> 1584698113 +0100
committer Jasper Hedegaard Bojsen <jasperhb@outlook.com> 1586933705 +0200

Find() with Cosmos DB
@AndriySvyryd AndriySvyryd merged commit b549dc6 into dotnet:master Apr 19, 2020
@AndriySvyryd
Copy link
Member

Regarding squashing the commits, I cannot get rid of the 3 rebase to master I did along the way, so unable to squash below 3 commits.

Good enough. Thanks for your contribution!

@1iveowl 1iveowl deleted the Optimize-find-with-partitionkey-generic branch April 20, 2020 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants