Skip to content

jdbc: set type of BigInteger as VARCHAR#3940

Merged
sougou merged 2 commits intovitessio:masterfrom
HubSpot:jdbc-big-int-fix
May 19, 2018
Merged

jdbc: set type of BigInteger as VARCHAR#3940
sougou merged 2 commits intovitessio:masterfrom
HubSpot:jdbc-big-int-fix

Conversation

@acharis
Copy link
Copy Markdown
Contributor

@acharis acharis commented May 16, 2018

Signed-off-by: Alex Charis acharis@hubspot.com

…INT64.

Signed-off-by: Alex Charis <acharis@hubspot.com>
@acharis acharis mentioned this pull request May 16, 2018
@harshit-gangal
Copy link
Copy Markdown
Member

harshit-gangal commented May 17, 2018

In Mysql Bigint can have negative value hence it was kept as INT64.
Min Value : -2^63
Max Value : 2^63-1

@acharis
Copy link
Copy Markdown
Contributor Author

acharis commented May 17, 2018

yeah, i'm not really sure what the right thing to do is...we're mapping a Java BigInteger, which provides arbitrary precision signed integers, onto a mysql bigint, which can be signed or unsigned and has limited precision. and i'm guessing we don't know which we're mapping to at this point in the code.

@harshit-gangal
Copy link
Copy Markdown
Member

@acharis Do you have reason for changing this to UINT64? Is it breaking things?
BigInteger can hold negative value which is same with Mysql.

We can think of sending it as string. Can you validate that.

@acharis
Copy link
Copy Markdown
Contributor Author

acharis commented May 18, 2018

Yes, things are broken as-is and this fixes them. Tested internally.

That being said, as above, I'm not sure what the right fix is.

Signed-off-by: Alex Charis <acharis@hubspot.com>
@harshit-gangal
Copy link
Copy Markdown
Member

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@harshit-gangal
Copy link
Copy Markdown
Member

LGTM

@acharis acharis changed the title Follow on to PR#3914, we set type of BigInteger as UINT64, not INT64 jdbc: set type of BigInteger as VARCHAR May 18, 2018
@sougou sougou merged commit 7b27a20 into vitessio:master May 19, 2018
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.

3 participants