-
Notifications
You must be signed in to change notification settings - Fork 877
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
Add AppendRows
helper
#1884
Add AppendRows
helper
#1884
Conversation
Seems reasonable at first glance. But it does make me wonder if combining the queries with |
That only works if the actual SQL shape is the same tho, whereas I can scan different things with different functions that both return a It might also be useful to preallocate or reuse a slice's space, by passing a zero-length slice with nonzero capacity, even when only collecting a single query's results. |
Good point. 👍 |
@@ -438,6 +436,11 @@ func CollectRows[T any](rows Rows, fn RowToFunc[T]) ([]T, error) { | |||
return slice, nil | |||
} | |||
|
|||
// CollectRows iterates through rows, calling fn for each row, and collecting the results into a slice of T. | |||
func CollectRows[T any](rows Rows, fn RowToFunc[T]) ([]T, error) { | |||
return AppendRows([]T(nil), rows, fn) |
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.
After updating from 5.5.1 to 5.5.3, our integration tests failed.
Diff:
--- Expected
+++ Actual
@@ -12,4 +12,3 @@
unknownFields: ([]uint8) <nil>,
- Edges: ([]*edgev1.Edge) {
- }
+ Edges: ([]*edgev1.Edge) <nil>
})
When SELECT * FROM table WHERE ...
returns no rows, 5.5.1 returns empty slice:
Line 424 in ba05097
slice := []T{} |
5.5.3 returns nil
slice:
Line 441 in 6f8f6ed
return AppendRows([]T(nil), rows, fn) |
@jackc I believe this is an unintentional change introduced in this PR.
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.
I don't think that CollectRows
was ever documented to return the dangling slice instead of the nil slice when no items were collected, but it's a simple enough fix (just change the call to AppendRows
to be AppendRows([]T{}, rows, fn)
).
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.
I don't think empty vs. nil slice was promised, but may as well preserve the previous behavior.
I found myself using
CollectRows
and appending the results immediately into a different slice, and I thought that a small tweak to the helper function to make it append to an existing slice would make it more useful in general.