Multiple enumerations of IEnumerable<SqlDataRecord> passed as TVP

[pvekshyn]

Looks like Dapper twice enumerates IEnumerable<SqlDataRecord> passed as TVP

var numbers = GenerateNumbers(3);
BulkInsert(numbers);

static IEnumerable<int> GenerateNumbers(int count)
{
    for (int i = 0; i < count; i++)
    {
        Console.WriteLine(i);
        yield return i;
    }
}

void BulkInsert(IEnumerable<int> numbers)
{
    var records = numbers.Select(x => MapToSqlDataRecord(x));
    var sql = @"INSERT INTO [Test] (Id) SELECT Id FROM @tvp;";

    using (var connection = new SqlConnection(connectionString))
    {
        connection.Execute(sql, new { tvp = records.AsTableValuedParameter("dbo.TestType") });
    }
}

static SqlDataRecord MapToSqlDataRecord(int i)
{
    var record = new SqlDataRecord(
        new SqlMetaData("Id", SqlDbType.Int)
    );
    record.SetInt32(0, i);
    return record;
}

this code writes to console 0 0 1 2

and if I pass enumerable from database it throws exception because can not open second reader on same connection (same enumerable works fine using ADO.Net)

I believe this is the first enumeration

[pvekshyn]

Ah, I got the idea, you are trying to avoid exception when the enumerable is empty. This is not correct, better catch exception in this case.

[mgravell]

I'm inclined to agree; at best we should type-test for IReadOnlyCollection<T> and perform this test via .Count; anything else: just send it down

[pvekshyn]

Let's say I have any source of enumerable: db reader, http returning IAsyncEnumerable, grpc streaming, whatever And I want to pass it to TVP without collecting the whole List in memory I can do it using ADO.Net, but it's not possible using Dapper because of second enumeration, and it will be definitely not possible with IReadOnlyCollection

[mgravell]

yes, I understand and agree; the point is that what we should change here is basically:

if (source is IReadOnlyList<T> { Count:= 0 })
{
    // short-circuit
}
// main code

In your case, the type-test will fail, so we'll just fall into the // main code, without doing any iteration.

[pvekshyn]

Great, thank you

P.S. on a second thought catching the exception is a bad idea, because sql will not be executed and caller should know about that