Sqlclient: Give SqlBulkCopy._rowsCopied a public getter

[stylesm]

Is your feature request related to a problem? Please describe.

SqlBulkCopy has a private member field for the number of rows copied (_rowsCopied), which is incremented with each record copy operation. Currently, to make use of this in client code, the solution is either to use the SqlRowsCopiedEventHandler (cumbersome and potentially expensive); or use reflection to get the value out of the private member field (hacky).

Describe the solution you'd like

Create a public property backed by _rowsCopied with a public getter - SqlBulkCopy.RowsCopied { get { return _rowsCopied; } }.

Or replace _rowsCopied with a public property with a public getter - SqlBulkCopy.RowsCopied { get; }

Describe alternatives you've considered

The SqlRowsCopiedEventHandler; using reflection to get the value out of the private member field; doing a SQL COUNT() before and after copying.

Additional context

Doesn't make sense to me to have a setter method for this property, but open to anyone's justification for it they think there should be!

Class in question: https://github.com/dotnet/SqlClient/blob/master/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs

[cheenamalhotra]

Hi @stylesm

Thanks for the suggestion.

Could you elaborate more on your use-case and is there a limitation in SqlRowsCopiedEventHandler that this API will address? Adding this API shouldn't be a problem, but we'd like to know more about any known limitations with SqlRowsCopiedEventHandler.

[cheenamalhotra]

@stylesm

We're waiting for your response to move forward with the PR.

Best Regards!

[chrisaliotta]

@cheenamalhotra I might not be fully informed here; however, I think the purpose of exposing this property is so the author does not have to handle the SqlRowsCopiedEvent in order to get a row count after the execution of a bulk copy.

It could be argued that under the current setup (the property being private), one could set the NotifyAfter property to something arbitrarily high in order to avoid multiple events being fired (from a performance point of view); although, this does seem somewhat of a non-optimal way of accomplishing the authors stated goals.

I'd imagine the user story to be something like this:

After a SqlBulkCopy I want to let the user know how many records were transferred for reporting and troubleshooting purposes.

[cheenamalhotra]

@chrisaliotta

We're on the same page, and that's the reason we're holding up on this one. There are many complaints about the SqlRowsCopiedEvent on StackOverflow, and users end up using Reflection, which can be handled by exposing this property.

But why would anyone use reflection is what I'm willing to hear. Is it just ease of code or limitation of SqlRowsCopiedEvent to trigger notification when it reaches a desired count.

[chrisaliotta]

@cheenamalhotra
Chances are, if they're using BulkCopy it's because they want to get the most performance possible for copying data. Attaching and handling an Event definitely slows down this process depending on how many times the event is fired or evaluated -- as such, it's a performance limitation.

I'd imagine that they use reflection _after_ the BulkCopy process rather than having to incur a performance hit of handling the events during the copy process. In other words, they don't care about tracking the progress; they just want to know at the end of the routine how many rows were copied.

Absent of handling the event, and storing the outcome, the best way to do this is through accessing the property by way of reflection (from what I understand).

[mikependon]

@cheenamalhotra - thank you for pushing this fix on v2.0. This is quite an important property exposure for SqlBulkCopy class that I mainly used on RepoDb.