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
Test and fix mapping of mysql and postgres native types #538
Changes from 12 commits
816142b
d69b568
6880d77
dba75e4
31a3357
1289552
90bebbc
4844732
4304b88
9d4c7f9
2b50140
e241cf4
b04fb09
63956d1
8d8ffad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
mod command_error; | ||
mod error; | ||
mod error_rendering; | ||
mod rpc; | ||
|
||
pub use error::Error; | ||
pub use rpc::RpcImpl; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ use quaint::{ | |
connector::ResultRow, | ||
}; | ||
use rust_decimal::{prelude::FromPrimitive, Decimal}; | ||
use std::{borrow::Borrow, io}; | ||
use std::{borrow::Borrow, io, str::FromStr}; | ||
use uuid::Uuid; | ||
|
||
/// An allocated representation of a `Row` returned from the database. | ||
|
@@ -136,7 +136,9 @@ pub fn row_value_to_prisma_value( | |
ParameterizedValue::Integer(i) => { | ||
PrismaValue::Float(Decimal::from_f64(i as f64).expect("f64 was not a Decimal.")) | ||
} | ||
ParameterizedValue::Text(s) => PrismaValue::Float(s.parse().unwrap()), | ||
ParameterizedValue::Text(_) | ParameterizedValue::Bytes(_) => { | ||
PrismaValue::Float(p_value.as_str().unwrap().parse().unwrap()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A necessary "Yo dawg! I heard you like unwrapping" meme... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, missed that one - we can probably do better error handling there. |
||
} | ||
_ => { | ||
let error = io::Error::new( | ||
io::ErrorKind::InvalidData, | ||
|
@@ -145,7 +147,23 @@ pub fn row_value_to_prisma_value( | |
return Err(SqlError::ConversionError(error.into())); | ||
} | ||
}, | ||
_ => PrismaValue::from(p_value), | ||
TypeIdentifier::Int => match p_value { | ||
ParameterizedValue::Integer(i) => PrismaValue::Int(i), | ||
ParameterizedValue::Bytes(bytes) => PrismaValue::Int(interpret_bytes_as_i64(&bytes)), | ||
ParameterizedValue::Text(txt) => PrismaValue::Int( | ||
i64::from_str(dbg!(txt.trim_start_matches('\0'))) | ||
.map_err(|err| SqlError::ConversionError(err.into()))?, | ||
), | ||
other => PrismaValue::from(other), | ||
}, | ||
TypeIdentifier::String => match p_value { | ||
ParameterizedValue::Uuid(uuid) => PrismaValue::String(uuid.to_string()), | ||
ParameterizedValue::Json(json_value) => { | ||
PrismaValue::String(serde_json::to_string(&json_value).expect("JSON value to string")) | ||
} | ||
ParameterizedValue::Null => PrismaValue::Null, | ||
other => PrismaValue::from(other), | ||
}, | ||
}) | ||
} | ||
|
||
|
@@ -171,3 +189,84 @@ impl From<&SqlId> for DatabaseValue<'static> { | |
id.clone().into() | ||
} | ||
} | ||
|
||
// We assume the bytes are stored as a big endian signed integer, because that is what | ||
// mysql does if you enter a numeric value for a bits column. | ||
fn interpret_bytes_as_i64(bytes: &[u8]) -> i64 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked already in slack. This makes me a bit worried due to this not being obvious in the type level that we're only talking about MySQL. I'm worried that somebody can without knowing write code that returns bytes from Quaint that are not from MySQL and we run this code and do some crazy wrong stuff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I'm not proposing or asking any changes, just writing down here that I'm a bit uncomfortable with this :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it would be nice to have a way to specialize things depending on the specific database we're working with in the sql query connector. That way we could do this, but only on mysql. This hack may go away when we expand the type system. |
||
match bytes.len() { | ||
8 => i64::from_be_bytes([ | ||
bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], | ||
]), | ||
len if len < 8 => { | ||
let sign_bit_mask: u8 = 0b10000000; | ||
// The first byte will only contain the sign bit. | ||
let most_significant_bit_byte = bytes[0] & sign_bit_mask; | ||
let padding = if most_significant_bit_byte == 0 { 0 } else { 0b11111111 }; | ||
let mut i64_bytes = [padding; 8]; | ||
|
||
for (target_byte, source_byte) in i64_bytes.iter_mut().rev().zip(bytes.iter().rev()) { | ||
*target_byte = *source_byte; | ||
} | ||
|
||
i64::from_be_bytes(i64_bytes) | ||
} | ||
0 => 0, | ||
_ => panic!("Attempted to interpret more than 8 bytes as an integer."), | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
|
||
#[test] | ||
fn quaint_bytes_to_integer_conversion_works() { | ||
// Negative i64 | ||
{ | ||
let i: i64 = -123456789123; | ||
let bytes = i.to_be_bytes(); | ||
let roundtripped = interpret_bytes_as_i64(&bytes); | ||
assert_eq!(roundtripped, i); | ||
} | ||
|
||
// Positive i64 | ||
{ | ||
let i: i64 = 123456789123; | ||
let bytes = i.to_be_bytes(); | ||
let roundtripped = interpret_bytes_as_i64(&bytes); | ||
assert_eq!(roundtripped, i); | ||
} | ||
|
||
// Positive i32 | ||
{ | ||
let i: i32 = 123456789; | ||
let bytes = i.to_be_bytes(); | ||
let roundtripped = interpret_bytes_as_i64(&bytes); | ||
assert_eq!(roundtripped, i as i64); | ||
} | ||
|
||
// Negative i32 | ||
{ | ||
let i: i32 = -123456789; | ||
let bytes = i.to_be_bytes(); | ||
let roundtripped = interpret_bytes_as_i64(&bytes); | ||
assert_eq!(roundtripped, i as i64); | ||
} | ||
|
||
// Positive i16 | ||
{ | ||
let i: i16 = 12345; | ||
let bytes = i.to_be_bytes(); | ||
let roundtripped = interpret_bytes_as_i64(&bytes); | ||
assert_eq!(roundtripped, i as i64); | ||
} | ||
|
||
// Negative i16 | ||
{ | ||
let i: i16 = -12345; | ||
let bytes = i.to_be_bytes(); | ||
let roundtripped = interpret_bytes_as_i64(&bytes); | ||
assert_eq!(roundtripped, i 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.
Next PR will test this (and things like non-utf8 encodings) to make sure we return good errors.