Use Postgres' RETURNING capability to optimize insertMany.
#407
Conversation
|
This should re-use functionality from Sql/Util.hs. In particular, this will fail at least for composite keys and should use |
|
Ok I added a failing test case for the composite primary key case, and then fixed that by adding a |
|
I actually haven't touched RawSql before, but it looks good now. Perhaps @snoyberg can look at the RawSql instance. |
|
One more thing, the documentation for |
| rawSqlCols _ key = (length $ keyToValues key, []) | ||
| rawSqlColCountReason key = "The primary key is composed of " | ||
| ++ (show $ length $ keyToValues key) | ||
| ++ " columns" |
There was a problem hiding this comment.
Is there a better way to write rawSqlColCountReason? Maybe a way to list out the components of the key?
* `RawSql` instance for `Key`s * Optimize `insertMany` for Postgres backend [ci skip]
|
@MaxGabriel The changes look good now, thanks for the great patch with the updated ChangeLog and documentation! @snoyberg this bumps persistent to 2.2 because the |
|
I'm OK skipping the major version bump, we typically do so for internal-only changes like this. |
Use Postgres' RETURNING capability to optimize `insertMany`.
|
hmm, I am having second thoughts on the minor version bump. If someone pegs their backend version, but not persistent, then re-installing dependencies can cause a breakage. If we do the major bump, then at least we can say persistent should have been constrained to a major version. @snoyberg are you ok with making this a major bump? |
|
Yes, I'm OK with it. Just to throw out one other possibility: we can add upper bounds via Hackage revisions to prevent the currently-released backends from using the newer persistent. I slightly prefer that approach, but only slightly. |
|
I tagged and released them all as version 2.2. That is enough work for me without changing a bunch of revisions on hackage. |
Closes #365.
This PR uses Postgres'
RETURNINGcapability to optimize bulk-inserts. Instead of inserting one-at-a-time, the rows can be inserted all at once and have their IDs returned. Based on my comment here, I don't think this is possible for SQLite/MySQL in a reliable way.I believe test coverage is already pretty good on this, because several tests use
insertMany, get back IDs, and then use those IDs to lookup the inserted rows.Things I'm unsure about:
InsertSqlResulttypes. IsISRManyKeyssupposed to be used when a composite primary key is defined?