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
Improve IAsyncEnumerable
usage
#4486
Conversation
/azp run test-all |
Azure Pipelines successfully started running 1 pipeline(s). |
Just for clarity when reviewing this PR, I'd like to point out the difference between the old and new signatures: // old
public static Task<TOutput[]> DeleteWithOutputAsync<TSource,TOutput>(
this IQueryable<TSource> source,
Expression<Func<TSource, TOutput>> outputExpression,
CancellationToken token = default)
// new methods
public static IAsyncEnumerable<TOutput> DeleteWithOutputAsync<TSource,TOutput>(
this IQueryable<TSource> source,
Expression<Func<TSource, TOutput>> outputExpression)
public static Task<TOutput[]> DeleteWithOutputAsync<TSource, TOutput>(
this IQueryable<TSource> source,
Expression<Func<TSource, TOutput>> outputExpression,
CancellationToken token) This means that the following code will immediately break compilation for v6 due to the cancellation token not being specified: // old syntax; immediately broken with v6
var ret = await query.DeleteWithOutputAsync(x => x.Id);
// required fix to use obsolete method
var ret = await query.DeleteWithOutputAsync(x => x.Id, default); As specified, however, the change is binary compatible - already-compiled code will continue to function with v6, as the optional parameter is built into the caller's compiled code and so will match on the obsolete method. UPDATE: this is inaccurate because |
1260c6d
to
d46fc0e
Compare
/azp run test-all |
Azure Pipelines successfully started running 1 pipeline(s). |
Test baselines changed by this PR. Don't forget to merge/close baselines PR after this pr merged/closed. |
@viceroypenguin , check mergeability with this PR #3401 |
@sdanyliv no merge conflicts; and shouldn't have any impact on #3401. |
/// <param name="sql">Command text.</param> | ||
/// <param name="parameter">Command parameter.</param> | ||
/// <returns>Async sequence of records returned by the query.</returns> | ||
public static IAsyncEnumerable<T> QueryToAsyncEnumerable<T>(this DataConnection connection, string sql, DataParameter parameter) |
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.
Why we have this overload for linq2db-mapped method?
/// <param name="sql">Command text.</param> | ||
/// <param name="parameters">Command parameters.</param> | ||
/// <returns>Async sequence of records returned by the query.</returns> | ||
public static IAsyncEnumerable<T> QueryToAsyncEnumerable<T>(this DataConnection connection, T template, string sql, params DataParameter[] parameters) |
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.
and why for template-based specification it is only 2 overloads?
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.
minor updates needed
934ef8e
This PR does two things:
InsertWithOutputAsync
,UpdateWithOutputAsync
, andDeleteWithOutputAsync
to returnIAsyncEnumerable
properly. This is a binary-compatible change, as the existing methods are kept. However, these methods are now marked with[Obsolete]
to encourage source migration to the newerIAsyncEnumerable
method.QueryToAsyncEnumerable
in places whereQueryToArrayAsync
andQueryToListAsync
are provided. This allows consumers to useIAsyncEnumerable
if desired. No obsoletions are planned here, though consumers are allowed and encouraged to upgrade if the query has many rows.💣 Breaking Change
XxxWithOutputAsync
are not source compatible, so this is a v6.0 breaking change. Consumers will need to add.ToListAsync()
or similar to address this change. This is to restore consistency betweenInsertWithOutputAsync
/UpdateWithOutputAsync
/DeleteWithOutputAsync
withMergeWithOutputAsync
; this consistency has been chosen to use the more flexible and modern approach.Replaces #4323