Skip to content

rows: Invalidate connection on error in discardResults() #513

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

Merged
merged 1 commit into from
Nov 9, 2016

Conversation

julienschmidt
Copy link
Member

Description

Previously the connection (mc) was not invalidated if an error occurred in discardResults(), which causes the driver to read on an invalid connection.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing

Fixes #422

@@ -700,10 +700,11 @@ func (rows *textRows) readRow(dest []driver.Value) error {
if data[0] == iEOF && len(data) == 5 {
// server_status [2 bytes]
rows.mc.status = readStatus(data[3:])
if err := rows.mc.discardResults(); err != nil {
err := rows.mc.discardResults()
rows.mc = nil
Copy link
Member

Choose a reason for hiding this comment

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

How about rows.mc.Close() before rows.mc = nil?
When discardResults() returned any error, I don't want to reuse the connection anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

PTAL

@julienschmidt julienschmidt force-pushed the invalidate-discardResults branch from ca68952 to a868ac0 Compare November 8, 2016 16:23
@julienschmidt julienschmidt force-pushed the invalidate-discardResults branch from a868ac0 to 0c50208 Compare November 8, 2016 16:27
Copy link
Member

@methane methane left a comment

Choose a reason for hiding this comment

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

LGTM

@julienschmidt julienschmidt merged commit 8f6c67f into master Nov 9, 2016
@julienschmidt julienschmidt deleted the invalidate-discardResults branch November 9, 2016 07:18
@florck florck mentioned this pull request Nov 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants