Skip to content

Fix appendLengthEncodedInteger for large numbers > 0xffffff #171

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 3 commits into from
Nov 11, 2013

Conversation

cxmcc
Copy link
Contributor

@cxmcc cxmcc commented Nov 11, 2013

If the value is ≥ (2^24) and < (2^64) it is stored as fe + 8-byte integer.

The current function seems to be ignoring this case xD.

@arnehormann
Copy link
Member

LGTM

@cxmcc
Copy link
Contributor Author

cxmcc commented Nov 11, 2013

There was a typo actually, I just amended it. Suddenly realized 7*8 != 54...

@arnehormann
Copy link
Member

Damn, I didn't catch that...
Please add a test (using an SQL statment).

@arnehormann
Copy link
Member

And thanks again, good catch!

@cxmcc
Copy link
Contributor Author

cxmcc commented Nov 11, 2013

@arnehormann
These two issues affect only:
Number of affected rows > 0xffffff
String length > 0xffffff (actually found this bug when trying length = 2^25)

Which is pretty big..It will be quite hard to test with SQL statements for these rare cases. We should probably do some unit tests for these single functions?

@arnehormann
Copy link
Member

Yes, you are right.
I'd like a roundtrip test with both functions in utils_test.go with the edge values of each possible range, then.

@julienschmidt
Copy link
Member

Not roundtrip tests again 😄
+1 for unit tests

@arnehormann
Copy link
Member

Roundtrip in this case: appendLengthEncodedInteger -> readLengthEncodedInteger, retrieve the exact same value that was entered. Not a database roundtrip.

@cxmcc
Copy link
Contributor Author

cxmcc commented Nov 11, 2013

@julienschmidt @arnehormann tests added.

@julienschmidt
Copy link
Member

LGTM

arnehormann added a commit that referenced this pull request Nov 11, 2013
Fix appendLengthEncodedInteger for large numbers > 0xffffff
@arnehormann arnehormann merged commit 789fdcb into go-sql-driver:master Nov 11, 2013
@cxmcc cxmcc deleted the lengthEncodedInteger branch November 11, 2013 21:13
@julienschmidt julienschmidt added this to the v1.2 milestone Apr 1, 2014
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.

3 participants