-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: implement unsigned int support for sqlite #919
Conversation
daead60
to
3338984
Compare
Thank you ❤️ |
|
||
impl<'q> Encode<'q, Sqlite> for u64 { | ||
fn encode_by_ref(&self, args: &mut Vec<SqliteArgumentValue<'q>>) -> IsNull { | ||
args.push(SqliteArgumentValue::Int64(*self as i64)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will result in data loss for u64
larger than std::i64::MAX
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be try_into()
but Encode
is not fallible (yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dignifiedquire Does your use case need u64
? It might be safer to remove this one until we have a fallible Encode
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, you are right. I am fine not having u64 supported for the moment, so likely better to remove this.
@mehcode there is still an issue with this code I am afraid, but I am not sure where it is coming from (sorry for not mentioning earlier here)
happens when I try to use this with #[derive(sqlx::Type)]
#[repr(u32)]
enum Origin {
Foo = 1,
Bar = 2,
// ...
} |
@mehcode any ideas on the above? |
@dignifiedquire I ran into that one recently, try using a signed integer instead ie. |
Thanks for pinging me. I'll look into this. |
No description provided.