-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: Detect partition key filters in queries #20690
Cosmos: Detect partition key filters in queries #20690
Conversation
...Core.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.QueryingEnumerable.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
Larger question, |
No, we should remove it. |
...Core.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.QueryingEnumerable.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
...Core.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.QueryingEnumerable.cs
Outdated
Show resolved
Hide resolved
Latest master
…ries # Conflicts: # src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs # src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs # src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.ReadItemQueryingEnumerable.cs # test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
...hapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
/// </summary> | ||
public virtual string GetPartitionKey([NotNull]IReadOnlyDictionary<string, object> parameterValues) | ||
{ | ||
return _partitionKeyProperty != null && _paritionKeyValueExpression is ConstantExpression constantExpression |
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.
_partitionKeyProperty is guaranteed not-null.
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.
Only if set by SetPartitionKeyProperty(..)
.
If SetPartitionKeyProperty
is never called it would be null, right?
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.
If _partitionKeyProperty
is null
you can return early
@@ -1012,6 +1029,156 @@ protected override ShapedQueryExpression TranslateWhere(ShapedQueryExpression so | |||
} | |||
|
|||
return null; | |||
|
|||
static (Expression predicateExpression, IProperty partitionKeyProperty, Expression partitionKeyValueExpression) |
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.
This is unnecessarily complicated. I will update it when merging.
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.
This is unnecessarily complicated. I will update it when merging.
Agree. Thanks for helping out.
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.
... also cleaned it up a bit in new latest commit.
@AndriySvyryd - Can you sign-off on this one? I will do cleanup and merge. |
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
…ingExpressionVisitor.cs Co-Authored-By: Andriy Svyryd <AndriySvyryd@users.noreply.github.com>
@1iveowl Thanks for another contribution! |
Suggestion for how to implement a partition key query detection filter.
Please see #20350