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

Fixed issue#3 #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed issue#3 #10

wants to merge 1 commit into from

Conversation

nnseva
Copy link

@nnseva nnseva commented Oct 16, 2013

The length of the mediumint type has been changed (3->4) accordingly to http://dev.mysql.com/doc/internals/en/binary-protocol-value.html#packet-ProtocolBinary::MYSQL_TYPE_INT24

The test case 1000_mediumint_fix_issue_3 has been added accordingly to user-reported cases.

@lukemarsden
Copy link

Hey Vsevolod, thanks for looking into this!

Do you know when the length of the mediumint type changed, ie in which version of MySQL?

If it happened quite recently, perhaps it's worth having a compatibility shim to detect which version of the server we're talking to and adjusting our behaviour accordingly.

If the change happened sufficiently long ago, I'd be happy to just state in the documentation for txMySQL that we don't support talking to sufficiently old MySQL servers.

My colleague @robhaswell is doing a style review, we'll be happy to merge this once the changes there have been addressed and we have an answer to the question above.

Thanks for volunteering your time to work on txMySQL!

Cheers,
Luke

@nnseva
Copy link
Author

nnseva commented Oct 16, 2013

Do you know when the length of the mediumint type changed, ie in which version of MySQL?

It looks like the mediumint field value length was the same in all MySQL versions::

  1. The documentation on http://dev.mysql.com/doc/internals/en/binary-protocol-value.html doesn't refer any version changes for this type
  2. issue#3 has been created 2 years ago and was caused particularly by messed length of mediumint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants