sql-kit icon indicating copy to clipboard operation
sql-kit copied to clipboard

Adds multirow insert method

Open NeedleInAJayStack opened this issue 3 years ago • 3 comments

This adds functionality to do a multi-row inserts with a single values method call.

Previously, to insert multiple rows a user had to call values repeatedly:

db.insert(into: "planets")
    .columns(["name", "color"])
    .values([SQLBind("Jupiter"), SQLBind("orange")])
    .values([SQLBind("Mars"), SQLBind("red")])
    .run()

This was a bit awkward when inserting rows from an array, where an instance of the builder had to be saved off and edited:

let rows: [[SQLExpression]]  = [[...], [...], ...]
let builder = db.insert(into: "planets")
    .columns(["name", "color"])
for row in rows {
    builder.values(row)
}
builder.run()

This MR simplifies the mutli-row insert situation by adding a values method overload that accepts a nested array:

db.insert(into: "planets")
    .columns(["name", "color"])
    .values([[SQLBind("Jupiter"), SQLBind("orange")], [SQLBind("Mars"), SQLBind("red")]])
    .run()

let rows  = [[...], [...], ...]
db.insert(into: "planets")
    .columns(["name", "color"])
    .values(rows)
    .run()

NOTE

This functionality was only added to the SQLExpression version of values, NOT the Encodable version of values. There are known issues with [Encodable] conforming to Encodable that prevent adequate type checks.

For example, if a values(_ rows: [[Encodable]]) is defined, it is undeterministic since the same call would also match values(_ rows: [Encodable]). If instead we change values(_ rows: [Encodable]) to detect [[Encodable]] cases at runtime, then it is difficult to guarantee that a caller hasn't mixed Encodable and [Encodable] values.

NeedleInAJayStack avatar Jul 29 '22 15:07 NeedleInAJayStack

Codecov Report

Merging #153 (c5b1f51) into main (a5c3df4) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head c5b1f51 differs from pull request most recent head e2ad748. Consider uploading reports for the commit e2ad748 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #153   +/-   ##
=======================================
  Coverage   69.27%   69.28%           
=======================================
  Files          94       94           
  Lines        3486     3487    +1     
=======================================
+ Hits         2415     2416    +1     
  Misses       1071     1071           
Flag Coverage Δ
unittests 69.28% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Sources/SQLKit/Builders/SQLInsertBuilder.swift 81.18% <100.00%> (+0.77%) :arrow_up:
Sources/SQLKit/Query/SQLQueryString.swift 89.65% <0.00%> (-0.51%) :arrow_down:

codecov-commenter avatar Jul 30 '22 05:07 codecov-commenter

welcome a bit of bikeshedding on the matter

I think multiValues(_:) is a fine name. Along the same vein, allValues seems like a reasonable option. However, I think I prefer rows(_:) or rowValues(_:), since I feel like they imply that all sequences should have equal length and correspond to the column ordering.

NeedleInAJayStack avatar Aug 03 '22 05:08 NeedleInAJayStack

@gwynne Sorry it took so long to get back to this one. I've adjusted it to use your implementation (which is great!), and named it rows. Again, I'm happy to switch to multiValues if you prefer that. Thanks!

NeedleInAJayStack avatar Jan 13 '23 06:01 NeedleInAJayStack