Skip to content

Commit

Permalink
Enable Multi Results support and discard additional results
Browse files Browse the repository at this point in the history
- packets.go: flag clientMultiResults, update status when receiving an
EOF packet, discard additional results on readRow when EOF is reached
- statement.go: currently a nil rows.mc is used as an eof, don’t set it
if there are no columns to avoid that Next() waits indefinitely
- rows.go: discard additional results on close and avoid panic on
Columns()
  • Loading branch information
lucalooz committed Apr 12, 2015
1 parent a197e5d commit 9542bdd
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 4 deletions.
45 changes: 43 additions & 2 deletions packets.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ func (mc *mysqlConn) writeAuthPacket(cipher []byte) error {
clientLongPassword |
clientTransactions |
clientLocalFiles |
clientMultiResults |
mc.flags&clientLongFlag

if mc.cfg.clientFoundRows {
Expand Down Expand Up @@ -470,6 +471,10 @@ func (mc *mysqlConn) handleErrorPacket(data []byte) error {
}
}

func readStatus(b []byte) statusFlag {
return statusFlag(b[0]) | statusFlag(b[1])<<8
}

// Ok Packet
// http://dev.mysql.com/doc/internals/en/generic-response-packets.html#packet-OK_Packet
func (mc *mysqlConn) handleOkPacket(data []byte) error {
Expand All @@ -484,7 +489,7 @@ func (mc *mysqlConn) handleOkPacket(data []byte) error {
mc.insertId, _, m = readLengthEncodedInteger(data[1+n:])

// server_status [2 bytes]
mc.status = statusFlag(data[1+n+m]) | statusFlag(data[1+n+m+1])<<8
mc.status = readStatus(data[1+n+m : 1+n+m+2])

// warning count [2 bytes]
if !mc.strict {
Expand Down Expand Up @@ -603,6 +608,11 @@ func (rows *textRows) readRow(dest []driver.Value) error {

// EOF Packet
if data[0] == iEOF && len(data) == 5 {
// server_status [2 bytes]
rows.mc.status = readStatus(data[3:])
if err := rows.mc.discardMoreResultsIfExists(); err != nil {
return err
}
rows.mc = nil
return io.EOF
}
Expand Down Expand Up @@ -660,6 +670,10 @@ func (mc *mysqlConn) readUntilEOF() error {
if err == nil && data[0] != iEOF {
continue
}
if err == nil && data[0] == iEOF && len(data) == 5 {
mc.status = readStatus(data[3:])
}

return err // Err or EOF
}
}
Expand Down Expand Up @@ -964,6 +978,28 @@ func (stmt *mysqlStmt) writeExecutePacket(args []driver.Value) error {
return mc.writePacket(data)
}

func (mc *mysqlConn) discardMoreResultsIfExists() error {
for mc.status&statusMoreResultsExists != 0 {
resLen, err := mc.readResultSetHeaderPacket()
if err != nil {
return err
}
if resLen > 0 {
// columns
if err := mc.readUntilEOF(); err != nil {
return err
}
// rows
if err := mc.readUntilEOF(); err != nil {
return err
}
} else {
mc.status &^= statusMoreResultsExists
}
}
return nil
}

// http://dev.mysql.com/doc/internals/en/binary-protocol-resultset-row.html
func (rows *binaryRows) readRow(dest []driver.Value) error {
data, err := rows.mc.readPacket()
Expand All @@ -973,11 +1009,16 @@ func (rows *binaryRows) readRow(dest []driver.Value) error {

// packet indicator [1 byte]
if data[0] != iOK {
rows.mc = nil
// EOF Packet
if data[0] == iEOF && len(data) == 5 {
rows.mc.status = readStatus(data[3:])
if err := rows.mc.discardMoreResultsIfExists(); err != nil {
return err
}
rows.mc = nil
return io.EOF
}
rows.mc = nil

// Error otherwise
return rows.mc.handleErrorPacket(data)
Expand Down
8 changes: 7 additions & 1 deletion rows.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type emptyRows struct{}

func (rows *mysqlRows) Columns() []string {
columns := make([]string, len(rows.columns))
if rows.mc.cfg.columnsWithAlias {
if rows.mc != nil && rows.mc.cfg.columnsWithAlias {
for i := range columns {
columns[i] = rows.columns[i].tableName + "." + rows.columns[i].name
}
Expand All @@ -61,6 +61,12 @@ func (rows *mysqlRows) Close() error {

// Remove unread packets from stream
err := mc.readUntilEOF()
if err == nil {
if err = mc.discardMoreResultsIfExists(); err != nil {
return err
}
}

rows.mc = nil
return err
}
Expand Down
2 changes: 1 addition & 1 deletion statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ func (stmt *mysqlStmt) Query(args []driver.Value) (driver.Rows, error) {
}

rows := new(binaryRows)
rows.mc = mc

if resLen > 0 {
rows.mc = mc
// Columns
// If not cached, read them and cache them
if stmt.columns == nil {
Expand Down

5 comments on commit 9542bdd

@knousere
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly the correction this driver needs. This solution enables use of stored procedures without compromising the integrity of the go-sql-driver/mysql package. Yes, there is a place for stored procedures in modern code. They allow for essential database performance enhancements and eliminate the need for ugly string pasting to make sql queries in go code.
I have been writing software for 35 years and see this solution as absolutely essential to maturing and completing this driver. So when will this be unified into the main thread? Do you need help making the case for unifying this fork?

@knousere
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Idhor My testing indicates that func statement.Exec() needs to capture the insertid when a stored procedure performs an insert. Currently, insertid comes back as 0 with err=nil

@lucalooz
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately i'm not using it in production yet so i'm doing those changes just when i discover that i need them
To be able to propose a pull request some tests should be written but i don't have much time so if someone writes them and does the pull request by copying my code i will not be angry ^^
Furthermore this change discards all additional results so it doesn't need non-standard driver methods
See go-sql-driver#66 (comment)

@julienschmidt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I'd like to use this commit in the main repository. Can you please make an entry into the AUTHORS file for copyright purposes?
Thank you!

@knousere
Copy link

@knousere knousere commented on 9542bdd Jun 4, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.