Add Rows and RowsContext methods to SelectStmt
#145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
could you describe how you process the rows? Just curious. |
|
There's an interface type with a method that returns a list of columns to
fetch and a list of pointers to scan them into. A bunch of types implement
this. I switched the query building to dbr, since it has a great query
builder, but to rewrite this particular functionality would require me to
rip out and re-architect a lot of the application which isn't really
something I can do right now (although it needs done eventually).
…On Sat, Sep 8, 2018, 7:42 PM taylorchu ***@***.***> wrote:
could you describe how you process the rows? Just curious.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#145 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIJHqB84ekIJuxKa1TUWx56haLkuh1fOks5uZFYBgaJpZM4Wf736>
.
|
taylorchu
reviewed
Sep 9, 2018
dbr.go
Outdated
|
|
||
| return rows, nil | ||
| } | ||
|
|
||
| func query(ctx context.Context, runner runner, log EventReceiver, builder Builder, d Dialect, dest interface{}) (int, error) { |
There was a problem hiding this comment.
Could you use queryRows in query too?
taylorchu
reviewed
Sep 9, 2018
select_test.go
Outdated
| count++ | ||
| } | ||
|
|
||
| require.Equal(t, 3, len(want)) |
There was a problem hiding this comment.
this should be require.Equal(t, len(want), count).
|
I initially did that, but query needs the text of the query that gets built
when it builds the final error message, and I didn't want to add extra
return values to queryRows to pass that information back to query. I'd be
happy to make the change if you'd prefer, though
…On Sun, Sep 9, 2018, 12:11 PM taylorchu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In dbr.go <#145 (comment)>:
> + defer func() {
+ log.TimingKv("dbr.select", time.Since(startTime).Nanoseconds(), kvs{
+ "sql": query,
+ })
+ }()
+
+ rows, err := runner.QueryContext(ctx, query, value...)
+ if err != nil {
+ return nil, log.EventErrKv("dbr.select.load.query", err, kvs{
+ "sql": query,
+ })
+ }
+
+ return rows, nil
+}
+
func query(ctx context.Context, runner runner, log EventReceiver, builder Builder, d Dialect, dest interface{}) (int, error) {
Could you use queryRows in query too?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#145 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIJHqBdnkJ4k9TBYlYpUB270tL_Yxy6Rks5uZT3IgaJpZM4Wf736>
.
|
|
It will be fine to return the query in |
|
I can't believe I missed that in the test - I was rushing before I left the house. I will get these changes made when I have a break today and push. |
- queryRows returns interpolated query - query calls queryRows - fixed erroneous check in test
|
Just got around to making the requested changes, sorry for the delay. Work stuff got in the way today. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm adding dbr to some legacy code right now, and until I have time to refactor some of the code to handle things differently, I've found myself wanting to build the query with dbr but process the rows differently. These are entirely convenience methods that allow fetching rows from a SelectStmt. They do not change any of the internals of dbr.