-
Notifications
You must be signed in to change notification settings - Fork 350
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
adding queryable extensions to DataServiceQuery<T> #2475
base: release-7.x
Are you sure you want to change the base?
Conversation
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
@@ -0,0 +1,1035 @@ | |||
//--------------------------------------------------------------------- | |||
// <copyright file="DataServiceQueryOfT.cs" company="Microsoft"> |
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.
/// Extension methods for <see cref="IOrderedQueryable{T}"/> | ||
/// </summary> | ||
/// <threadsafety static="true"/> | ||
public static class OrderedQueryableExtensions |
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.
any reason why do we add this in the Microsoft.Spatial project.
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.
+1
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public static IOrderedQueryable<T> AsOrderedQueryable<T>(this IOrderedQueryable<T> queryable) | ||
{ | ||
return queryable; |
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.
public static IQueryable<T> AsQueryable<T>(this IQueryable<T> queryable) | ||
{ | ||
return queryable; | ||
} |
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.
any reason why do we add this in the Microsoft.Spatial project.
@@ -0,0 +1,24 @@ | |||
namespace Microsoft.Spatial | |||
{ |
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.
copyright
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.
Can we have some unit or E2E tests on how these extensions can be used. E.g in ExecuteBatch
as described in the issue.
/// Extension methods for <see cref="IOrderedQueryable{T}"/> | ||
/// </summary> | ||
/// <threadsafety static="true"/> | ||
public static class OrderedQueryableExtensions |
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.
+1
@@ -0,0 +1,1035 @@ | |||
//--------------------------------------------------------------------- | |||
// <copyright file="DataServiceQueryOfT.cs" company="Microsoft"> |
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.
// <copyright file="DataServiceQueryOfT.cs" company="Microsoft"> | |
// <copyright file="DataServiceQueryExtensions.cs" company="Microsoft"> | |
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Linq.Expressions; | ||
|
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.
using System.Linq; | ||
using System.Linq.Expressions; | ||
|
||
using Microsoft.Spatial; |
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.
We shouldn't be having extensions in the spatial project.
Issues
This pull request fixes #2409.
Description
From the issue, when a
DataServiceQuery<T>
is used as anIQueryable<T>
for LINQ queries, the returned object isIQueryable<T>
. This means that it cannot be passed toDataServiceContext.ExecuteBatch
. To facilitate this, we will now have custom extension methods forDataServiceQuery<T>
which are analogs to those extensions onIQueryable<T>
, except they returnDataServiceQuery<T>
variants. This will allow them to still be used inDataServiceContext.ExecuteBatch
. An explanation of the options discussed can be found here.