-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
tidb: Fix panic in driverStmt.Query #63
Conversation
The prepared statement maybe not query, so the resultset maybe nil.
c.Assert(err, IsNil) | ||
_, err = db.Exec("insert into t values (1), (2), (3)") | ||
c.Assert(err, IsNil) | ||
_, err = db.Query("delete from `t` where `c` = ?", 1) |
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.
Query should returns an empty row for delete.
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.
PTAL
Address comment
Address comment
1. Close() 2. Call Next() two times
PTAL |
should this can fix #57 ? |
@@ -379,6 +388,9 @@ func newdriverRows(rs rset.Recordset) *driverRows { | |||
// result is inferred from the length of the slice. If a particular column | |||
// name isn't known, an empty string should be returned for that entry. | |||
func (r *driverRows) Columns() []string { | |||
if r.rs == nil { | |||
return make([]string, 0) |
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.
can return nil here?
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.
if return an empty slice, using []string{} is better?
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 am not sure. I think return empty array is better than nil.
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.
[]string{} +1
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.
PTAL
I am not sure. I run go-xorm tidb test with another error. So I can not reach the panic point the same as lunny. |
LGTM |
tidb: Fix panic in driverStmt.Query
… duration (pingcap#63) * restore: extend the closeWithRetry() wait duration to 30 seconds * kv-deliver: properly close the write stream before recyling the client
* impl Restore of ColunmNameExpr
* impl Restore of ColunmNameExpr
* impl Restore of ColunmNameExpr
Signed-off-by: disksing <[email protected]>
…pingcap#51449) (pingcap#51) (pingcap#63) close pingcap#51448 Co-authored-by: 山岚 <[email protected]>
The prepared statement maybe not query, so the resultset maybe nil.