vapor / sql-kit

*️⃣ Build SQL queries in Swift. Extensible, protocol-based design that supports DQL, DML, and DDL.
MIT License
248 stars 58 forks source link

Adds multirow insert method #153

Closed NeedleInAJayStack closed 2 months ago

NeedleInAJayStack commented 2 years ago

These changes are now available in 3.32.0

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.

codecov-commenter commented 2 years ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (3c8c6aa) to head (e2b955b). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #153 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 100 100 Lines 2635 2636 +1 ========================================= + Hits 2635 2636 +1 ``` | [Files with missing lines](https://app.codecov.io/gh/vapor/sql-kit/pull/153?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor) | Coverage Δ | | |---|---|---| | [...it/Builders/Implementations/SQLInsertBuilder.swift](https://app.codecov.io/gh/vapor/sql-kit/pull/153?src=pr&el=tree&filepath=Sources%2FSQLKit%2FBuilders%2FImplementations%2FSQLInsertBuilder.swift&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9TUUxLaXQvQnVpbGRlcnMvSW1wbGVtZW50YXRpb25zL1NRTEluc2VydEJ1aWxkZXIuc3dpZnQ=) | `100.00% <100.00%> (ø)` | |
NeedleInAJayStack commented 2 years ago

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 commented 1 year ago

@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!

gwynne commented 2 months ago

@NeedleInAJayStack Any chance you'd be able to update this to resolve the conflicts? I'd be willing to merge it as SQLKit 3's last new feature.

NeedleInAJayStack commented 2 months ago

Sure, I can have that done in the next day or so if that works

NeedleInAJayStack commented 2 months ago

Hey @gwynne, I've resolved the merge conflicts and updated the tests to the new test formatting. I wasn't sure if any of the tests in the original PR were obsoleted by new work, so just let me know if you'd like any removed/organized differently. Thanks!