-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
add missing QueryUnbufferedAsync<T> API #1912
Conversation
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.
👍 Thanks for tweaks!
I see a new release for this but nothing published to nuget, is this planned to be pushed? |
It is there. If you use the version number from the release notes it should
work.
…On Mon, 12 Jun 2023, 16:47 Michael Plaisted, ***@***.***> wrote:
I see a new release for this but nothing published to nuget, is this
planned to be pushed?
—
Reply to this email directly, view it on GitHub
<#1912 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMBTRHW2BBV42VUNP5DXK42Z7ANCNFSM6AAAAAAZAW2OMU>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
|
||
var func = tuple.Func; | ||
|
||
var convertToType = Nullable.GetUnderlyingType(effectiveType) ?? effectiveType; |
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 convertToType
variable is not used. I guess it was meant to be used 4 lines below?
yield return GetValue<T>(reader, convertToType, val);
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.
You're almost certainly correct; thanks, good eyes! I'll try to look at this tomorrow (I guess I need some tests that do QueryUnbufferedAsync<int>
, QueryUnbufferedAsync<int?>
, QueryUnbufferedAsync<string>
, QueryUnbufferedAsync<SomeValueType>
and QueryUnbufferedAsync<SomeClass>
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.
Or maybe it should just be deleted because apparently the convertToType
is already computed inside GetValue<T>(…)
.
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.
added for tracking: #1920 (because already merged)
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.
/// A sequence of data of <typeparamref name="T"/>; if a basic type (int, string, etc) is queried then the data from the first column is assumed, otherwise an instance is | ||
/// created per row, and a direct column-name===member-name mapping is assumed (case insensitive). | ||
/// </returns> | ||
public static IAsyncEnumerable<T> QueryUnbufferedAsync<T>(this DbConnection cnn, string sql, object param = null, DbTransaction transaction = null, int? commandTimeout = null, CommandType? commandType = 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.
Is there a reason this is using a DbConnection
and not an IDbConnection
?
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.
Async doesn't work on IDbConnection, it never has. We should have done this with the old methods but can't break things now - IDbConnection simply predates async being a thing, so it never had support. The methods needed are on the abstract base class.
I am not proposing to add the myriad of multi-read
<T0, T1, ..., T5>
APIs that go with this