Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it safe to reuse a SqlDataRecord?

When implementing table-valued parameters, one of the most common ways to generate an IEnumerable<SqlDataRecord> for use by the parameter is code like this (e.g., https://stackoverflow.com/a/10779567/18192 ):

public static IEnumerable<SqlDataRecord> Rows(List<int> simpletable)
{
    var smd = new []{ new SqlMetaData("id", SqlDbType.Int)};
    var sqlRow = new SqlDataRecord(smd);
    foreach (int i in simpletable)
    {
        sqlRow.SetInt32(0, i);
        yield return sqlRow;
    }
}
//...
var param = sqlCmd.Parameters.AddWithValue("@retailerIDs", Rows(mydata)); 
param.SqlDbType = SqlDbType.Structured;
param.TypeName = "myTypeName";

This code does seem to work. While reusing SqlMetaData does not set off too many alarm bells, declaring the SqlDataRecord outside the foreach loop feels incredibly suspicious to me:

A mutable object is modified and then yielded repeatedly.

As an example of why this is concerning, calling var x = Rows(new[] { 100, 200}.ToList()).ToList().Dump() in LinqPad spits out 200,200. This approach seems to rely on an implementation detail (that rows are processed individually), but I don't see any documentation which promises this.

Is there some mitigating factor which renders this approach safe?

like image 933
Brian Avatar asked May 03 '16 14:05

Brian


2 Answers

This approach seems to rely on an implementation detail (that rows are processed individually), but I don't see any documentation which promises this.

Is there some mitigating factor which renders this approach safe?

As user1249190 points out, Reusing SQLDataRecord is explicitly recommended in the remarks section of https://docs.microsoft.com/en-us/dotnet/api/microsoft.sqlserver.server.sqldatarecord#remarks :

This class is used together with SqlPipe to send result sets to the client from managed code stored-procedures. When writing common language runtime (CLR) applications, you should re-use existing SqlDataRecord objects instead of creating new ones every time. Creating many new SqlDataRecord objects could severely deplete memory and adversely affect performance.

Obviously this recommendation does not apply to usage across threads: The documentation also explicitly warns that "Any instance members are not guaranteed to be thread safe."

like image 148
Brian Avatar answered Sep 22 '22 05:09

Brian


If you don't need it outside of the foreach loop at all, I don't see why you would want to re-use it.

I found this question Is there a reason for C#'s reuse of the variable in a foreach? which links to this answer in another question Is it better coding practice to define variables outside a foreach even though more verbose? where Jon Skeet answered saying:

There's no advantage to declaring the variables outside the loop, unless you want to maintain their values between iterations.

(Note that usually this makes no behavioural difference, but that's not true if the variables are being captured by a lambda expression or anonymous method.)

like image 41
Zack Avatar answered Sep 22 '22 05:09

Zack