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

Support for 64-bit integers #43

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mfp
Copy link

@mfp mfp commented Dec 3, 2021

This PR adds support for 64-bit integer fields and param binding.
I was able to test signed integers (full range from -9223372036854775808 to 9223372036854775807), but not unsigned integers yet -- the code is very similar though, making it less of a leap of faith.

Even though they compile, I was not able to use the stress tests: they segfault in the unmodified tree as of 862877a

Credits

The work on this PR was performed in the context of Ahrefs open-source work day:
https://twitter.com/javierwchavarri/status/1466774912148910088

@paurkedal
Copy link
Collaborator

I agree this is a good addition (CC @tatchi who mentioned it in #61). I don't have write access to the branch of this PR, so I've pushed my suggestion to [my own 64bit-integers branch](https://github.com/paurkedal/ocaml-mariadb/commits/64bit-integers/]:

  • I merged the current master branch (avoiding some conflict resolving of a rebase).
  • My third commit allows some extra conversions, which for int may overflow.

The rationale for the possibly overflowing conversion of int is simplicity and backwards compatibility, see also my commit message. We could consider raising an exception in case of overflow.

If you want to add more commits, can you merge in (parts of) my branch first, or rebase in cherry pick?

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