Add Multi-Results support #537
Conversation
|
Does this patch support query like this? How about removing |
I double checked, currently it works only with selects, I will fix it.
Good point, thanks! |
- support for binary protocol - support statements returning no results - remove emptyRows
| } | ||
|
|
||
| if rows.NextResultSet() { | ||
| dbt.Error("unexpected next result set") |
There was a problem hiding this comment.
Why next result set is unexpected?
What happens for DO 1; DO 2; DO 3; SELECT 42?
I expect
if !rows.NextResultSet() { dbt.Error("expect 4 resultset, got 1") }
if !rows.NextResultSet() { dbt.Error("expect 4 resultset, got 2") }
if !rows.NextResultSet() { dbt.Error("expect 4 resultset, got 3") }
var v int64
if err := rows.Scan(&v); err != nil {
dbt.Error(err)
}
if v != 42 {
dbt.Error("expected 42; got ", v)
}There was a problem hiding this comment.
I followed the example from the std:
https://tip.golang.org/pkg/database/sql/#example_DB_Query_multipleResultSets
which seems to ignore the empty results sets.
There was a problem hiding this comment.
So what would happen if I have SELECT 1 FROM DUAL WHERE 1 = 0; SELECT 1,2 FROM DUAL WHERE 1 = 1;? That is the first query returns no rows, but the second query does.
I would expect
if !rows.NextResultSet() { dbt.Error("expect 2 resultset, got 1") }
That is, I might not know which query has no resultset. So, I have to go through them all to map processing them correctly.
There was a problem hiding this comment.
@joegrasse, sorry - "empty results sets" was inaccurate. Result set is present if column count > 0.
So, in the example that you provided it would work as you expected - it would return two result sets, first would be empty, the other one would have one row.
There was a problem hiding this comment.
Query() and NextResultSet() has very similar code.
It seems it can be exported to method later.
But it's OK for first implementation.
LGTM, except recursion.
Go optimimze tail call only in very limited situation.
I prefer simple loop over tail call recursion.
| if resLen == 0 { | ||
| rows.rs.done = true | ||
| return rows.NextResultSet() | ||
| } |
There was a problem hiding this comment.
I don't like recursion.
Would you rewrite this to loop?
There was a problem hiding this comment.
sure, I just pushed a fix
|
|
||
| if resLen == 0 { | ||
| rows.rs.done = true | ||
| return rows.NextResultSet() |
* rows.Close() would hang on readUntilEOF if some results were ignored before calling NextResultSet()
- get rid of a recursive call
|
Any chance of getting this merged soon now that go1.8 has been released (which adds support for this)? Thanks! |
|
This cause Travis test fail: https://travis-ci.org/methane/mysql/jobs/214213219 |
|
I'm sorry, it was my mistake. |
Fixes #420