Skip to content
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

defer getWarnings() after fetching resultsets. #609

Closed
wants to merge 2 commits into from

Conversation

methane
Copy link
Member

@methane methane commented Jun 6, 2017

Description

Don't query "SHOW WARNINGS" in handleOkPacket(). Defer it to end of query.

fixes #602.

(Since Travis doesn't update #605 status, I recreate this pullrequest).

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

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

This also should have a test

connection.go Outdated
@@ -31,6 +31,7 @@ type mysqlConn struct {
sequence uint8
parseTime bool
strict bool
warningCount uint16
Copy link
Member

Choose a reason for hiding this comment

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

@julienschmidt julienschmidt added this to the v1.4 milestone Jun 6, 2017
@methane
Copy link
Member Author

methane commented Jun 6, 2017

Now I'm +1 to remove strict mode.
After reading protocol specification, I realized current implementation is incomplete.

see this document

Current code handles only warningCount in OK packet.
But warningCount is in EOF packet too. strict mode ignores it.

And keeping "strict mode" increase maintenance cost significantly.

julienschmidt added a commit that referenced this pull request Sep 29, 2017
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.

Incorrect packet parsing for multiStatement=true
2 participants