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?
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."
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.)
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With