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

Allows for the retrievel of more than 2^31-1 rows in a single query. #863

Merged
merged 3 commits into from Jul 28, 2020
Merged

Allows for the retrievel of more than 2^31-1 rows in a single query. #863

merged 3 commits into from Jul 28, 2020

Conversation

akamor
Copy link
Contributor

@akamor akamor commented Jul 27, 2020

Prior to this fix the internal counter for the sequence number was stored as a signed 32-bit integer which was causing overflow issues when we hit Int32.MaxValue.

Signed-off-by: Your Name adam@tonic.ai

Prior to this fix the internal counter for the sequence number was stored as a signed 32-bit integer which was causing overflow issues when we hit Int32.MaxValue.

Signed-off-by: Adam Kamor <adam@tonic.ai>
@bgrainger
Copy link
Member

bgrainger commented Jul 28, 2020

Thanks for the PR! One minor (legal) thing: can you please amend the commit message so the sign-off has the form Signed-off-by: Your Real Name <adam@tonic.ai> with your actual name? I do need to make sure I have the rights to use all contributions to the repository.

You can edit the commit message with git commit --amend (or GUI tools provide various ways to do it).

@bgrainger bgrainger linked an issue Jul 28, 2020 that may be closed by this pull request
akamor added 2 commits Jul 28, 2020
Prior to this fix the internal counter for the sequence number was stored as a signed 32-bit integer which was causing overflow issues when we hit Int32.MaxValue.

Signed-off-by: Adam Kamor <adam@tonic.ai>
@akamor
Copy link
Contributor Author

akamor commented Jul 28, 2020

Done

image

@akamor
Copy link
Contributor Author

akamor commented Jul 28, 2020

FYI, I'm attempting to spin up a 2B+ row table to confirm this is working as expected. Would be good to know there aren't other places in code that are gonna have overflow issues. I should have results in a few hours.

@akamor
Copy link
Contributor Author

akamor commented Jul 28, 2020

@bgrainger Just finished a success test with 2B+ rows (a bit more than 2^31-1 actually) and there were no issues.

FWIW, however, I know the sequence number references the packet # and not the actually number of rows. The table I used was a single column of 32bit integers which likely has some affect on the # of packets sent. Just throwing it out there in case it somehow matters.

@bgrainger
Copy link
Member

bgrainger commented Jul 28, 2020

Excellent, thanks!

@akamor
Copy link
Contributor Author

akamor commented Jul 28, 2020

Updated my comment above right after you replied so just making sure you see my update.

@bgrainger bgrainger merged commit 6cf3459 into mysql-net:master Jul 28, 2020
2 checks passed
@bgrainger
Copy link
Member

bgrainger commented Jul 28, 2020

MySQL Server sends one row per (MySQL) packet: https://dev.mysql.com/doc/internals/en/com-query-response.html#packet-ProtocolText::ResultsetRow

(These may be bundled together into one network/TCP packet but they're logically separate. So the counter will overflow after 2.1 billion rows.)

@bgrainger
Copy link
Member

bgrainger commented Aug 13, 2020

Fixed in 1.0.1.

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

Successfully merging this pull request may close these issues.

Query fails when returning more than Int32Max rows on v1.0.0
2 participants