-
Notifications
You must be signed in to change notification settings - Fork 976
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: use RETURNING clause for batch create #3293
Conversation
f07edcd
to
d01ce4c
Compare
Added some comments, all tests pass now. Should be good to go! |
Codecov Report
@@ Coverage Diff @@
## master #3293 +/- ##
==========================================
- Coverage 77.64% 77.57% -0.07%
==========================================
Files 325 325
Lines 20511 20570 +59
==========================================
+ Hits 15925 15958 +33
- Misses 3367 3386 +19
- Partials 1219 1226 +7
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Small nit
persistence/sql/batch/create.go
Outdated
@@ -38,7 +42,7 @@ type ( | |||
} | |||
) | |||
|
|||
func buildInsertQueryArgs[T any](ctx context.Context, quoter quoter, models []*T) insertQueryArgs { | |||
func buildInsertQueryArgs[T any](ctx context.Context, dialect string, mapper *reflectx.Mapper, quoter quoter, models []*T) (insertQueryArgs, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildInsertQueryArgs
never returns an error I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, probably an artifact form playing around with reflection. Will fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
batch.Create
regressed on gobuffalo/pop improvements that useRETURNING
clauses andgen_random_uuid()
. This patch addresses thatChecklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
[email protected]) from the maintainers to push
the changes.
works.