Dapper icon indicating copy to clipboard operation
Dapper copied to clipboard

QueryAsync cannot insert multiple items and return a single record set.

Open KeithHenry opened this issue 6 years ago • 8 comments

I want to add multiple records to a table and get back the IDs, something like this:

var addedIDs = await connection.QueryAsync<int>(@"
    insert into myTable (PropA, ProbB)
    output inserted.MyID
    values (@propA, @propB)", 
    items.Select( x => new { propA = x.A, propB = x.B });

That throws an exception from this line: https://github.com/StackExchange/Dapper/blob/80231b4d2b2391bbb01c2cd7be75fbecfe0c0296/Dapper/SqlMapper.cs#L1686

This appears to be very deliberately applied to stop this from working (including a test in https://github.com/StackExchange/Dapper/commit/bb5a16ce7abc6468da741f33303d2212f2652d60)

It's not clear why this is blocked or what the correct behaviour should be, as I expect multiple records in my result set, so ExecuteAsync is no use - at best I get the last ID added only.

I think this may also be the cause of https://github.com/StackExchange/Dapper/issues/930

KeithHenry avatar Mar 05 '18 19:03 KeithHenry

Incidentally, if I spin out the values extension that I expect SqlMapper to do, something like this:

var p = new DynamicParameters();
var values = new List<string>();
int i = 0;
foreach (var x in items) {
    values.Add($"(@a{i}, @b{i})");
    p.Add($"a{i}", x.A);
    p.Add($"b{i}", x.B);
    i++;
}

var addedIDs = await connection.QueryAsync<int>($@"
    insert into myTable (PropA, ProbB)
    output inserted.MyID
    values {(string.Join(",", values.ToArray()))}", p);

It works fine. There is a workaround, but I'm not sure why it's needed?

KeithHenry avatar Mar 05 '18 19:03 KeithHenry

Hi. I have the same problem. @KeithHenry thank you for the workaround. However if there are too much data then you may encounter server constraints (for example, I got exception about maximum parameter count - 2100). So I implemented splitting on parts. @mgravell could you explain please, is it correct behaviour? I mean throwing exception https://github.com/StackExchange/Dapper/blob/80231b4d2b2391bbb01c2cd7be75fbecfe0c0296/Dapper/SqlMapper.cs#L1686 when we need to get multiple ids of inserted records. May be are there correct solutions?

grishinalexey avatar Sep 06 '18 10:09 grishinalexey

@grishinalexey Yeah, my workaround example is trivial - whether this bug is fixed or not you'll have to batch operations involving >2100 params.

I think it's been added as without the output the insert returns a rowcount, which isn't suitable for QueryAsync - there are probably a lot more confused users complaining that QueryAsync doesn't work with a rowcount than there are users asking for output support.

KeithHenry avatar Sep 06 '18 17:09 KeithHenry

I don't think we can guarantee the order of Ids matches the Order or items passed to QueryAsync, so the Ids are not returned so you don't try and assign them back to the objects out of order.

Also I assume output is not provider agnostic.

rhubley avatar Sep 06 '18 17:09 rhubley

@rhubley output is a T-SQL keyword, I don't know whether other providers support it. It's not the keyword though - Dapper assumes that enumerable parameters will never be valid for enumerable outputs, which is wrong. output is just a specific case where it should allow this pattern.

Basically:

var addedIDs = await connection.QueryAsync<int>(@"Any query I want", arrayOfThings);

Should be allowed by the framework, and indeed was before https://github.com/StackExchange/Dapper/commit/bb5a16ce7abc6468da741f33303d2212f2652d60 was applied.

KeithHenry avatar Sep 07 '18 15:09 KeithHenry

For the Execute(...) case, Dapper ends up breaking enumerable parameters into multiple commands. Each round trip is actually executed sequentially unless you remember to include CommandFlags.Pipelined (surprising default behavior).

I for one wasn't aware that concurrent commands could be "pipelined" on the same connection. As a workaround, it's not too hard to do something similar that returns data.

Example:

var users = this.GetUsersToInsert();
var cxn = this.GetDbConnection();

const string sql = "INSERT INTO users (name, address) VALUES (@name, @address) OUTPUT inserted.id";
var insertedIds = await Task.WhenAll(
    users.Select(x => cxn.ExecuteScalarAsync<int>(sql, new { x.name, x.address })));

kmcclellan avatar May 18 '22 17:05 kmcclellan

I don't that all connection objects support multiple async operations active on the same connection. other users should be aware this can cause issues.

rhubley avatar May 18 '22 18:05 rhubley

For the Execute(...) case, Dapper ends up breaking enumerable parameters into multiple commands. Each round trip is actually executed sequentially unless you remember to include CommandFlags.Pipelined (surprising default behavior).

I for one wasn't aware that concurrent commands could be "pipelined" on the same connection. As a workaround, it's not too hard to do something similar that returns data.

Example:

var users = this.GetUsersToInsert();
var cxn = this.GetDbConnection();

const string sql = "INSERT INTO users (name, address) VALUES (@name, @address) OUTPUT inserted.id";
var insertedIds = await Task.WhenAll(
    users.Select(x => cxn.ExecuteScalarAsync<int>(sql, new { x.name, x.address })));

This worked for me, although I do agree with labeling it a workaround. Would love to get some thoughts on this challenge from the Dapper team.

I can faintly here the voice of Eric Lippert crying out that a Linq is for querying, not updating, as he succinctly pointed out in the accepted answer here, that is also cross referenced here.

That being said, you got to do what you got to do. The solution accepted here seems to assume you're only ever inserting 1 row at a time, or that you only want the most recently generated ID. Your workaround overcomes those limitations, and I thank you for it!

segunak avatar Jun 13 '22 23:06 segunak