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

SqlBulkCopy: Add RowsCopied Property #136

Closed
fowl2 opened this issue Jul 9, 2019 · 3 comments
Closed

SqlBulkCopy: Add RowsCopied Property #136

fowl2 opened this issue Jul 9, 2019 · 3 comments
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain.

Comments

@fowl2
Copy link

fowl2 commented Jul 9, 2019

It is is difficult to find out how many rows SqlBulkCopy.WriteToServer[Async] has copied. This is a problem when using streaming as DbDataReader doesn't provide this information either.

Describe the solution you'd like

class SqlBulkCopy
{
...
  public long RowsCopied { get; }
...
}

It would be cleaner to change the return type of WriteToServer[Async] to long, but I think this would technically be a breaking change? This would also have the advantage of not having to worry about multiple calls or reads before writing.

Describe alternatives you've considered

  • setting NotifyAfter to 1, subscribing to SqlRowsCopied and then storing it somewhere from the event handler. Cons: overhead of calling a delegate, plus the allocation of EventArgs per row.
  • fully buffer to a DataTable. Cons: whole dataset in memory.
  • wrapping IDataReader and keeping count of the number of times Read() is called. Cons:
    doesn't work for async readers.
  • wrapping DbDataReader. Cons: lots of boilerplate, can't call protected methods on the source leading to unintended behaviour divergence.
  • SELECT COUNT(*) FROM DESTINATION. Cons: network round trip, concurrency issues.
  • Reflecting on the private _rowsCopied field. Cons: reflection, brittle.

Additional context

@cheenamalhotra cheenamalhotra added the 💡 Enhancement Issues that are feature requests for the drivers we maintain. label Jul 9, 2019
@cheenamalhotra
Copy link
Member

Hi @fowl2

Thanks for bringing up the request. We'll investigate more and come back to you!

@cheenamalhotra cheenamalhotra added this to the Future milestone Jul 9, 2019
@juliusfriedman
Copy link

This is a good suggestion, we currently use reflection to access this field because we don't wan't to have the overhead of an event for every row copied just to get the count.

@cheenamalhotra
Copy link
Member

@fowl2 @juliusfriedman

#404 is same issue we addressed with PR #409 .
The API will be part of next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Enhancement Issues that are feature requests for the drivers we maintain.
Projects
None yet
Development

No branches or pull requests

3 participants