Test and fix mapping of mysql and postgres native types #538
Conversation
tomhoule
force-pushed
the
query-engine/test-database-types-support
branch
3 times, most recently
from
05a1608
to
e35e764
Compare
tomhoule
force-pushed
the
query-engine/test-database-types-support
branch
3 times, most recently
from
1388b6c
to
41c636b
Compare
tomhoule
force-pushed
the
query-engine/test-database-types-support
branch
4 times, most recently
from
6fc0279
to
2128ed1
Compare
tomhoule
changed the title
tomhoule
force-pushed
the
query-engine/test-database-types-support
branch
from
b6a4b08
to
dc12321
Compare
tomhoule
changed the title
tomhoule
force-pushed
the
query-engine/test-database-types-support
branch
7 times, most recently
from
411a83c
to
6b29c05
Compare
It maps them to an integer, assuming they were populated with an integer value, but this could not be the case.
It appears to be a bug in rust_decimal. Issue link: paupino/rust-decimal#228
tomhoule
force-pushed
the
query-engine/test-database-types-support
branch
from
cb18d43
to
9d4c7f9
Compare
| @@ -15,6 +15,9 @@ impl<'a> From<ParameterizedValue<'a>> for PrismaValue { | |||
| ParameterizedValue::Uuid(uuid) => PrismaValue::Uuid(uuid), | |||
| ParameterizedValue::DateTime(dt) => PrismaValue::DateTime(dt), | |||
| ParameterizedValue::Char(c) => PrismaValue::String(c.to_string()), | |||
| ParameterizedValue::Bytes(bytes) => PrismaValue::String( | |||
| String::from_utf8(bytes.into_owned()).expect("PrismaValue::String from ParameterizedValue::Bytes"), | |||
There was a problem hiding this comment.
Next PR will test this (and things like non-utf8 encodings) to make sure we return good errors.
| @@ -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.
A necessary "Yo dawg! I heard you like unwrapping" meme...
There was a problem hiding this comment.
Oh, missed that one - we can probably do better error handling there.
|
|
||
| // 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.
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.
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.
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.
| @@ -33,6 +33,7 @@ impl ToSqlRow for ResultRow { | |||
| fn to_sql_row<'b>(self, idents: &[(TypeIdentifier, FieldArity)]) -> crate::Result<SqlRow> { | |||
| let mut row = SqlRow::default(); | |||
| let row_width = idents.len(); | |||
| row.values.reserve(row_width); | |||
There was a problem hiding this comment.
Hmm. What is the difference between this and then implementing with_capacity to SqlRow and using it?
There was a problem hiding this comment.
I think it's the same.
This is a first PR. I will test more in follow-ups.