Repository navigation
x/sqlbuilder/upserter: add MultiExec - #47
AlexisMontagne wants to merge 2 commits into
Conversation
upfluence-bot
added
reviewed/needs-review
reviewed/needs-work
and removed
reviewed/needs-review
labels
c910255 to
d527dc5
Compare
Upserting many rows one by one costs a round trip per row, and a serializable transaction of their own without the key constraint. MultiExecer executes a batch of rows at once, all or none of them: with a single INSERT ... ON CONFLICT for the key constrained Insert and Upsert modes, each row updating with its own values, and in one transaction otherwise. It is discovered with a type assertion, as the key constrained Update mode, the key constrained Upsert mode with a PredicateClause and the statements with Returning cannot implement it.
d527dc5 to
d56e4f5
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unconstrained batching through InTxUpserter can leave partial writes after an error, violating the all-or-none guarantee.
1 open issue
What changed in this PR
Adds optional batch execution to x/sqlbuilder/upserter, reducing transaction overhead while preserving existing single-row execution.
Changes:
- Introduces
MultiExecerwith a single insert for constrained batches and a shared transaction for unconstrained batches. - Refactors row execution and hides batch support for returning statements and constrained updates.
- Adds tests for batch results and capability detection.
| File | Description |
|---|---|
| x/sqlbuilder/upserter/upserter_test.go | Tests batch insert, update, upsert, and unsupported configurations. |
| x/sqlbuilder/upserter/execer.go | Implements batch execution and shared row-processing helpers. |
Review effort: Balanced
d527dc5 to
d56e4f5
Compare
upfluence-bot
added
reviewed/needs-review
reviewed/needs-work
and removed
reviewed/needs-review
reviewed/needs-work
labels
jlevesy
left a comment
jlevesy
left a comment
There was a problem hiding this comment.
👀
Sounds good
I'm not fully sure to wrap my head around everything. Would appreciate a walkthrough.
| ), | ||
| stmt.SetValues..., | ||
| ), | ||
| Table: stmt.Table, |
There was a problem hiding this comment.
NIt: could be nice to split this function into newQueryConstrainedExecer and newInPlaceExecer or something like that.
upfluence-bot
added
reviewed/needs-work
and removed
reviewed/needs-review
reviewed/needs-work
labels
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| Where: stmt.PredicateClause, | ||
| }, | ||
| ) | ||
| Returning: stmt.Returning, |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
SA1019: (github.com/upfluence/sql/x/sqlbuilder.InsertStatement).Returning is deprecated: Please use Returnings (staticcheck)
| }, | ||
| Action: sqlbuilder.Nothing, | ||
| }, | ||
| Returning: stmt.Returning, |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
SA1019: (github.com/upfluence/sql/x/sqlbuilder.InsertStatement).Returning is deprecated: Please use Returnings (staticcheck)
What does this PR do?
Upserting many rows one by one costs a round trip per row, plus a serializable transaction of their own without the key constraint.
This adds
MultiExecertox/sqlbuilder/upserter, which executes a batch of rows at once, all or none of them:INSERT ... ON CONFLICTfor the key constrainedInsertandUpsertmodes, each row updating with its own values (viaExcluded)What are the observable changes?
Feature: statements built by the upserter can be executed for many rows in one call. The capability is discovered with a type assertion:
The key constrained
Updatemode and the statements withReturningdo not implement it.Good PR checklist
Additional Notes
INSERTis bound by the database parameter limit, as the rows are not chunked.InsertExecer.MultiExecand theExcludedmarker, both already onmaster.