-
Notifications
You must be signed in to change notification settings - Fork 61
fix(mariadb): json gt/gte/lt/lte comparisons #315
Conversation
fn value_into_json(value: &Value) -> Option<serde_json::Value> { | ||
match value.clone() { | ||
// MariaDB returns JSON as text | ||
Value::Text(Some(text)) => { | ||
let json: serde_json::Value = | ||
serde_json::from_str(&text).expect(format!("expected parsable text to json, found {}", text).as_str()); | ||
|
||
Some(json) | ||
} | ||
Value::Json(Some(json)) => Some(json), | ||
_ => None, | ||
} | ||
} |
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.
I ended up using an external helper and not adding a match to Value::as_json()
because it's a const method and serde_json::from_str
is not const
so it cannot work. And changing the const method to a normal method I believe is a breaking change that wasn't worth it.
fn json_to_quaint_value<'a>(json: serde_json::Value) -> crate::Result<Value<'a>> { | ||
match json { | ||
serde_json::Value::String(str) => Ok(Value::text(str)), | ||
serde_json::Value::Number(number) => { | ||
if let Some(int) = number.as_i64() { | ||
Ok(Value::integer(int)) | ||
} else if let Some(float) = number.as_f64() { | ||
Ok(Value::double(float)) | ||
} else { | ||
unreachable!() | ||
} | ||
} | ||
x => { | ||
let msg = format!("Expected JSON string or number, found: {}", x); | ||
let kind = ErrorKind::conversion(msg.clone()); | ||
|
||
let mut builder = Error::builder(kind); | ||
builder.set_original_message(msg); | ||
|
||
Err(builder.build()) | ||
} | ||
} | ||
} |
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 is the actual fix. We no longer use CAST (? AS JSON)
but instead render the values deserialized, which works both on MySQL & MariaDB
(left, right) if left.is_json_value() && right.is_json_extract_fun() => { | ||
self.surround_with("CAST(", " AS JSON)", |s| s.visit_expression(left))?; | ||
let quaint_value = json_to_quaint_value(left.into_json_value().unwrap())?; |
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.
Usage of the function above
Overview
Follow-up PR to #311
It turns out
CAST(? as JSON)
is not supported by MariaDB. We work around this limitation by no longer rendering a JSON value compared toJSON_EXTRACT()
as JSON, but as its parsed type.As a result though, it is no longer possible to perform numeric comparisons with
JSON_EXTRACT()
as the left/right and a JSON value as the left/right member that's NOT a number or a string.Examples
Additional notes
Since we added MySQL8 and MariaDB to the list of tested databases, some failing tests have appeared. I created an issue to track this: #318