From c7b831515e9d0bf42609b3ece740b0c93ef84061 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Sun, 12 Jan 2025 22:29:48 +0100 Subject: [PATCH 01/16] feat: initial json_object implementation --- core/function.rs | 2 ++ core/translate/expr.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/core/function.rs b/core/function.rs index 7b906406a..e74842046 100644 --- a/core/function.rs +++ b/core/function.rs @@ -29,6 +29,7 @@ pub enum JsonFunc { JsonArrowExtract, JsonArrowShiftExtract, JsonExtract, + JsonObject, JsonType, } @@ -45,6 +46,7 @@ impl Display for JsonFunc { Self::JsonArrayLength => "json_array_length".to_string(), Self::JsonArrowExtract => "->".to_string(), Self::JsonArrowShiftExtract => "->>".to_string(), + Self::JsonObject => "json_object".to_string(), Self::JsonType => "json_type".to_string(), } ) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 2f88c9a0d..5f0ca446c 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -129,6 +129,24 @@ macro_rules! expect_arguments_min { }}; } +macro_rules! expect_arguments_even { + ( + $args:expr, + $func:ident + ) => {{ + let args = $args.as_deref().unwrap_or_default(); + if args.len() % 2 != 0 { + crate::bail_parse_error!( + "{} function requires an even number of arguments", + $func.to_string() + ); + }; + // The only function right now that requires an even number is `json_object` and it allows + // to have no arguments, so thats why in this macro we do not bail with teh `function with no arguments` error + args + }}; +} + pub fn translate_condition_expr( program: &mut ProgramBuilder, referenced_tables: &[TableReference], @@ -812,6 +830,18 @@ pub fn translate_expr( func_ctx, ) } + JsonFunc::JsonObject => { + let args = expect_arguments_even!(args, j); + + translate_function( + program, + &args, + referenced_tables, + resolver, + target_register, + func_ctx, + ) + } }, Func::Scalar(srf) => { match srf { From 3785e7c7f811ba1de552948f361192093bdec7c2 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Mon, 13 Jan 2025 01:09:48 +0100 Subject: [PATCH 02/16] feat: initial json_object implementation --- core/function.rs | 2 ++ core/json/mod.rs | 30 ++++++++++++++++++++++++++++++ core/vdbe/mod.rs | 14 +++++++++++--- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/core/function.rs b/core/function.rs index e74842046..d147cf318 100644 --- a/core/function.rs +++ b/core/function.rs @@ -380,6 +380,8 @@ impl Func { #[cfg(feature = "json")] "json_extract" => Ok(Func::Json(JsonFunc::JsonExtract)), #[cfg(feature = "json")] + "json_object" => Ok(Func::Json(JsonFunc::JsonObject)), + #[cfg(feature = "json")] "json_type" => Ok(Func::Json(JsonFunc::JsonType)), "unixepoch" => Ok(Self::Scalar(ScalarFunc::UnixEpoch)), "julianday" => Ok(Self::Scalar(ScalarFunc::JulianDay)), diff --git a/core/json/mod.rs b/core/json/mod.rs index f0a362ab6..4028c036a 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -288,6 +288,36 @@ pub fn json_type(value: &OwnedValue, path: Option<&OwnedValue>) -> crate::Result Ok(OwnedValue::Text(LimboText::json(Rc::new(val.to_string())))) } +// TODO: document this +pub fn json_object(values: &[OwnedValue]) -> crate::Result { + let value_map = values + .chunks(2) + .map(|chunk| match chunk { + [key, value] => { + let key = match key { + // TODO: is this tp_string call ok? + OwnedValue::Text(t) => t.value.to_string(), + // TODO: I matched sqlite message error here. Is this ok? + _ => crate::bail_constraint_error!("labels must be TEXT"), + }; + + // TODO: check the part about interpreting json if the value comes from another json_object function + // FIXME: right now, this statement `select json_object('a','{"a":2}');` differs from sqlite3. + // We should only interpret json if it comes from a json function. The same goes to json_array. + // TODO: inspire from json_array func + let json_value = get_json_value(value)?; + + Ok((key, json_value)) + } + _ => crate::bail_constraint_error!("json_object requires an even number of values"), + }) + // TODO: collecting into a IndexMap does not allow for repeated keys + .collect::, _>>()?; + + let result = crate::json::to_string(&value_map).unwrap(); + Ok(OwnedValue::Text(LimboText::json(Rc::new(result)))) +} + /// Returns the value at the given JSON path. If the path does not exist, it returns None. /// If the path is an invalid path, returns an error. /// diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 64c41b3bd..370e8d252 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -28,6 +28,7 @@ use crate::error::{LimboError, SQLITE_CONSTRAINT_PRIMARYKEY}; #[cfg(feature = "uuid")] use crate::ext::{exec_ts_from_uuid7, exec_uuid, exec_uuidblob, exec_uuidstr, ExtFunc, UuidFunc}; use crate::function::{AggFunc, FuncCtx, MathFunc, MathFuncArity, ScalarFunc}; +use crate::json::json_object; use crate::pseudo::PseudoCursor; use crate::result::LimboResult; use crate::schema::Table; @@ -1352,12 +1353,19 @@ impl Program { } } #[cfg(feature = "json")] - crate::function::Func::Json(JsonFunc::JsonArray) => { + crate::function::Func::Json( + func @ (JsonFunc::JsonArray | JsonFunc::JsonObject), + ) => { let reg_values = &state.registers[*start_reg..*start_reg + arg_count]; - let json_array = json_array(reg_values); + let func = match func { + JsonFunc::JsonArray => json_array, + JsonFunc::JsonObject => json_object, + _ => unreachable!(), + }; + let json_result = func(reg_values); - match json_array { + match json_result { Ok(json) => state.registers[*dest] = json, Err(e) => return Err(e), } From e01b8f8343b1b0c76a5bc7bc1bfecc4aa4b314de Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Mon, 13 Jan 2025 01:26:44 +0100 Subject: [PATCH 03/16] feat: initial json_object implementation --- core/json/mod.rs | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 4028c036a..a27979f58 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -83,6 +83,8 @@ pub fn json_array(values: &[OwnedValue]) -> crate::Result { let mut s = String::new(); s.push('['); + // TODO: use `convert_db_type_to_json` and map each value with that function, + // so we can construct a `Val::Array` with each value and then serialize it directly. for (idx, value) in values.iter().enumerate() { match value { OwnedValue::Blob(_) => crate::bail_constraint_error!("JSON cannot hold BLOB values"), @@ -253,6 +255,27 @@ fn convert_json_to_db_type(extracted: &Val, all_as_db: bool) -> crate::Result` for `Val` here, instead of this? +// the same with `convert_json_to_db_type`... +fn convert_db_type_to_json(value: &OwnedValue) -> crate::Result { + let val = match value { + OwnedValue::Null => Val::Null, + OwnedValue::Float(f) => Val::Float(*f), + OwnedValue::Integer(i) => Val::Integer(*i), + OwnedValue::Text(t) => match t.subtype { + // Convert only to json if the subtype is json (got it from another json function) + TextSubtype::Json => get_json_value(value)?, + TextSubtype::Text => { + let text = t.value.to_string(); + Val::String(text) + } + }, + // TODO: is this error ok? + _ => crate::bail_constraint_error!("JSON cannot hold BLOB values"), + }; + Ok(val) +} + pub fn json_type(value: &OwnedValue, path: Option<&OwnedValue>) -> crate::Result { if let OwnedValue::Null = value { return Ok(OwnedValue::Null); @@ -301,13 +324,13 @@ pub fn json_object(values: &[OwnedValue]) -> crate::Result { _ => crate::bail_constraint_error!("labels must be TEXT"), }; - // TODO: check the part about interpreting json if the value comes from another json_object function - // FIXME: right now, this statement `select json_object('a','{"a":2}');` differs from sqlite3. - // We should only interpret json if it comes from a json function. The same goes to json_array. - // TODO: inspire from json_array func - let json_value = get_json_value(value)?; + // TODO: with `convert_db_type_to_json` we convert the value to JSON only if + // it was obtained from another JSON function. + // TODO: the `json_array` function should use something like this. + // That implementation could be a lot simpler with this function + let json_val = convert_db_type_to_json(value)?; - Ok((key, json_value)) + Ok((key, json_val)) } _ => crate::bail_constraint_error!("json_object requires an even number of values"), }) From 06208af2e870a66969d334fc04abd049df95331c Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Mon, 13 Jan 2025 01:29:32 +0100 Subject: [PATCH 04/16] chore: add todo --- core/json/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/json/mod.rs b/core/json/mod.rs index a27979f58..758737815 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -319,6 +319,8 @@ pub fn json_object(values: &[OwnedValue]) -> crate::Result { [key, value] => { let key = match key { // TODO: is this tp_string call ok? + // TODO: We can construct the IndexMap from Rc, but we must enable the + // serde's `rc` feature so we can serialize Rc OwnedValue::Text(t) => t.value.to_string(), // TODO: I matched sqlite message error here. Is this ok? _ => crate::bail_constraint_error!("labels must be TEXT"), From 20566919a418fece7b7a23a8c25e2221b39b6620 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Mon, 13 Jan 2025 23:19:57 +0100 Subject: [PATCH 05/16] fix: compilation in CI --- core/vdbe/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index eb44827ea..ba9ee1697 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -28,7 +28,6 @@ use crate::error::{LimboError, SQLITE_CONSTRAINT_PRIMARYKEY}; #[cfg(feature = "uuid")] use crate::ext::{exec_ts_from_uuid7, exec_uuid, exec_uuidblob, exec_uuidstr, ExtFunc, UuidFunc}; use crate::function::{AggFunc, FuncCtx, MathFunc, MathFuncArity, ScalarFunc}; -use crate::json::json_object; use crate::pseudo::PseudoCursor; use crate::result::LimboResult; use crate::storage::sqlite3_ondisk::DatabaseHeader; @@ -41,7 +40,7 @@ use crate::vdbe::insn::Insn; use crate::{ function::JsonFunc, json::get_json, json::json_array, json::json_array_length, json::json_arrow_extract, json::json_arrow_shift_extract, json::json_error_position, - json::json_extract, json::json_type, + json::json_extract, json::json_object, json::json_type, }; use crate::{Connection, Result, Rows, TransactionState, DATABASE_VERSION}; use datetime::{exec_date, exec_datetime_full, exec_julianday, exec_time, exec_unixepoch}; From a4cab8e628c23de66e2d44fb3cbee7d2c0a6a15b Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Mon, 13 Jan 2025 23:20:15 +0100 Subject: [PATCH 06/16] test: add tests for json_object --- core/json/mod.rs | 213 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 180 insertions(+), 33 deletions(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 256513428..4d61fae0e 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -257,6 +257,7 @@ fn convert_json_to_db_type(extracted: &Val, all_as_db: bool) -> crate::Result` for `Val` here, instead of this? // the same with `convert_json_to_db_type`... fn convert_db_type_to_json(value: &OwnedValue) -> crate::Result { @@ -272,7 +273,6 @@ fn convert_db_type_to_json(value: &OwnedValue) -> crate::Result { Val::String(text) } }, - // TODO: is this error ok? _ => crate::bail_constraint_error!("JSON cannot hold BLOB values"), }; Ok(val) @@ -313,38 +313,6 @@ pub fn json_type(value: &OwnedValue, path: Option<&OwnedValue>) -> crate::Result Ok(OwnedValue::Text(LimboText::json(Rc::new(val.to_string())))) } -// TODO: document this -pub fn json_object(values: &[OwnedValue]) -> crate::Result { - let value_map = values - .chunks(2) - .map(|chunk| match chunk { - [key, value] => { - let key = match key { - // TODO: is this tp_string call ok? - // TODO: We can construct the IndexMap from Rc, but we must enable the - // serde's `rc` feature so we can serialize Rc - OwnedValue::Text(t) => t.value.to_string(), - // TODO: I matched sqlite message error here. Is this ok? - _ => crate::bail_constraint_error!("labels must be TEXT"), - }; - - // TODO: with `convert_db_type_to_json` we convert the value to JSON only if - // it was obtained from another JSON function. - // TODO: the `json_array` function should use something like this. - // That implementation could be a lot simpler with this function - let json_val = convert_db_type_to_json(value)?; - - Ok((key, json_val)) - } - _ => crate::bail_constraint_error!("json_object requires an even number of values"), - }) - // TODO: collecting into a IndexMap does not allow for repeated keys - .collect::, _>>()?; - - let result = crate::json::to_string(&value_map).unwrap(); - Ok(OwnedValue::Text(LimboText::json(Rc::new(result)))) -} - /// Returns the value at the given JSON path. If the path does not exist, it returns None. /// If the path is an invalid path, returns an error. /// @@ -453,6 +421,38 @@ pub fn json_error_position(json: &OwnedValue) -> crate::Result { } } +// TODO: document this +pub fn json_object(values: &[OwnedValue]) -> crate::Result { + let value_map = values + .chunks(2) + .map(|chunk| match chunk { + [key, value] => { + let key = match key { + // TODO: is this to_string call ok? + // TODO: We can construct the IndexMap from Rc, but we must enable the + // serde's `rc` feature so we can serialize Rc + OwnedValue::Text(t) => t.value.to_string(), + // TODO: I matched sqlite message error here. Is this ok? + _ => crate::bail_constraint_error!("labels must be TEXT"), + }; + + // TODO: with `convert_db_type_to_json` we convert the value to JSON only if + // it was obtained from another JSON function. + // TODO: the `json_array` function should use something like this. + // That implementation could be a lot simpler with this function + let json_val = convert_db_type_to_json(value)?; + + Ok((key, json_val)) + } + _ => crate::bail_constraint_error!("json_object requires an even number of values"), + }) + // TODO: collecting into a IndexMap does not allow for repeated keys + .collect::, _>>()?; + + let result = crate::json::to_string(&value_map).unwrap(); + Ok(OwnedValue::Text(LimboText::json(Rc::new(result)))) +} + #[cfg(test)] mod tests { use super::*; @@ -837,4 +837,151 @@ mod tests { let result = json_error_position(&input).unwrap(); assert_eq!(result, OwnedValue::Integer(16)); } + + #[test] + fn test_json_object_simple() { + let key = OwnedValue::build_text(Rc::new("key".to_string())); + let value = OwnedValue::build_text(Rc::new("value".to_string())); + let input = vec![key, value]; + + let result = json_object(&input).unwrap(); + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!(json_text.value.as_str(), r#"{"key":"value"}"#); + } + + #[test] + fn test_json_object_multiple_values() { + let text_key = OwnedValue::build_text(Rc::new("text_key".to_string())); + let text_value = OwnedValue::build_text(Rc::new("text_value".to_string())); + let json_key = OwnedValue::build_text(Rc::new("json_key".to_string())); + let json_value = OwnedValue::Text(LimboText::json(Rc::new( + r#"{"json":"value","number":1}"#.to_string(), + ))); + let integer_key = OwnedValue::build_text(Rc::new("integer_key".to_string())); + let integer_value = OwnedValue::Integer(1); + let float_key = OwnedValue::build_text(Rc::new("float_key".to_string())); + let float_value = OwnedValue::Float(1.1); + + let input = vec![ + text_key, + text_value, + json_key, + json_value, + integer_key, + integer_value, + float_key, + float_value, + ]; + + let result = json_object(&input).unwrap(); + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!( + json_text.value.as_str(), + r#"{"text_key":"text_value","json_key":{"json":"value","number":1},"integer_key":1,"float_key":1.1}"# + ); + } + + #[test] + fn test_json_object_json_value_is_rendered_as_json() { + let key = OwnedValue::build_text(Rc::new("key".to_string())); + let value = OwnedValue::Text(LimboText::json(Rc::new(r#"{"json":"value"}"#.to_string()))); + let input = vec![key, value]; + + let result = json_object(&input).unwrap(); + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!(json_text.value.as_str(), r#"{"key":{"json":"value"}}"#); + } + + #[test] + fn test_json_object_json_text_value_is_rendered_as_regular_text() { + let key = OwnedValue::build_text(Rc::new("key".to_string())); + let value = OwnedValue::Text(LimboText::new(Rc::new(r#"{"json":"value"}"#.to_string()))); + let input = vec![key, value]; + + let result = json_object(&input).unwrap(); + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!( + json_text.value.as_str(), + r#"{"key":"{\"json\":\"value\"}"}"# + ); + } + + #[test] + fn test_json_object_nested() { + let key = OwnedValue::build_text(Rc::new("key".to_string())); + let value = OwnedValue::build_text(Rc::new("value".to_string())); + let input = vec![key, value]; + + let parent_key = OwnedValue::build_text(Rc::new("parent_key".to_string())); + let parent_value = json_object(&input).unwrap(); + let parent_input = vec![parent_key, parent_value]; + + let result = json_object(&parent_input).unwrap(); + + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!( + json_text.value.as_str(), + r#"{"parent_key":{"key":"value"}}"# + ); + } + // TODO: is this behaviour ok? it differs from sqlite + #[test] + fn test_json_object_duplicated_keys() { + let key = OwnedValue::build_text(Rc::new("key".to_string())); + let value = OwnedValue::build_text(Rc::new("value".to_string())); + let input = vec![key.clone(), value.clone(), key, value]; + + let result = json_object(&input).unwrap(); + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!(json_text.value.as_str(), r#"{"key":"value"}"#); + } + + #[test] + fn test_json_object_empty() { + let input = vec![]; + + let result = json_object(&input).unwrap(); + let OwnedValue::Text(json_text) = result else { + panic!("Expected OwnedValue::Text"); + }; + assert_eq!(json_text.value.as_str(), r#"{}"#); + } + + #[test] + fn test_json_object_non_text_key() { + let key = OwnedValue::Integer(1); + let value = OwnedValue::build_text(Rc::new("value".to_string())); + let input = vec![key, value]; + + match json_object(&input) { + Ok(_) => panic!("Expected error for non-TEXT key"), + Err(e) => assert!(e.to_string().contains("labels must be TEXT")), + } + } + + #[test] + fn test_json_odd_number_of_values() { + let key = OwnedValue::build_text(Rc::new("key".to_string())); + let value = OwnedValue::build_text(Rc::new("value".to_string())); + let input = vec![key.clone(), value, key]; + + match json_object(&input) { + Ok(_) => panic!("Expected error for odd number of values"), + Err(e) => assert!(e + .to_string() + .contains("json_object requires an even number of values")), + } + } } From fea8d19359c8499b2d5e2c0c27a8ccde01a245cc Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Tue, 14 Jan 2025 00:00:31 +0100 Subject: [PATCH 07/16] test: add TCL tests for json_object --- core/json/mod.rs | 6 +++++- testing/json.test | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 4d61fae0e..de97c8f78 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -863,6 +863,8 @@ mod tests { let integer_value = OwnedValue::Integer(1); let float_key = OwnedValue::build_text(Rc::new("float_key".to_string())); let float_value = OwnedValue::Float(1.1); + let null_key = OwnedValue::build_text(Rc::new("null_key".to_string())); + let null_value = OwnedValue::Null; let input = vec![ text_key, @@ -873,6 +875,8 @@ mod tests { integer_value, float_key, float_value, + null_key, + null_value, ]; let result = json_object(&input).unwrap(); @@ -881,7 +885,7 @@ mod tests { }; assert_eq!( json_text.value.as_str(), - r#"{"text_key":"text_value","json_key":{"json":"value","number":1},"integer_key":1,"float_key":1.1}"# + r#"{"text_key":"text_value","json_key":{"json":"value","number":1},"integer_key":1,"float_key":1.1,"null_key":null}"# ); } diff --git a/testing/json.test b/testing/json.test index 758243bcd..ea1c9bf0f 100755 --- a/testing/json.test +++ b/testing/json.test @@ -506,3 +506,41 @@ do_execsql_test json_error_position_null { do_execsql_test json_error_position_complex { SELECT json_error_position('{a:null,{"h":[1,[1,2,3]],"j":"abc"}:true}'); } {{9}} + +do_execsql_test json_object_simple { + SELECT json_object('key', 'value'); +} {{{"key":"value"}}} + +do_execsql_test json_object_nested { + SELECT json_object('grandparent',json_object('parent', json_object('child', 'value'))); +} {{{"grandparent":{"parent":{"child":"value"}}}}} + +do_execsql_test json_object_quoted_json { + SELECT json_object('parent', '{"child":"value"}'); +} {{{"parent":"{\"child\":\"value\"}"}}} + +do_execsql_test json_object_unquoted_json { + SELECT json_object('parent', json('{"child":"value"}')); +} {{{"parent":{"child":"value"}}}} + +do_execsql_test json_object_multiple_values { + SELECT json_object('text', 'value', 'json', json_object('key', 'value'), 'int', 1, 'float', 1.5, 'null', null); +} {{{"text":"value","json":{"key":"value"},"int":1,"float":1.5,"null":null}}} + +do_execsql_test json_object_empty { + SELECT json_object(); +} {{{}}} + +do_execsql_test json_object_json_array { + SELECT json_object('ex',json('[52,3]')); +} {{{"ex":[52,3]}}} + +do_execsql_test json_from_json_object { + SELECT json(json_object('key','value')); +} {{{"key":"value"}}} + +# FIXME: this behaviour differs from sqlite. Although, sqlite docs states +# that this could change in a "future enhancement" (https://www.sqlite.org/json1.html#jobj) +#do_execsql_test json_object_duplicated_keys { +# SELECT json_object('key', 'value', 'key', 'value2'); +#} {{{"key":"value2"}}} From b3d3c8cf6570a14f4d3eb1f21907cb587fafeff6 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Tue, 14 Jan 2025 00:04:40 +0100 Subject: [PATCH 08/16] docs: update COMPAT.md --- COMPAT.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/COMPAT.md b/COMPAT.md index 5584899f4..36fa84a11 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -229,7 +229,7 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html). ### Date and Time Modifiers | Modifier | Status| Comment | |----------------|-------|---------------------------------| -| Days | Yes | | +| Days | Yes | | | Hours | Yes | | | Minutes | Yes | | | Seconds | Yes | | @@ -270,7 +270,7 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html). | json ->> path | Yes | | | json_insert(json,path,value,...) | | | | jsonb_insert(json,path,value,...) | | | -| json_object(label1,value1,...) | | | +| json_object(label1,value1,...) | Yes | When keys are duplicated, only the last one processed is returned. This differs from sqlite, where the keys in the output can be duplicated | | jsonb_object(label1,value1,...) | | | | json_patch(json1,json2) | | | | jsonb_patch(json1,json2) | | | From cd16a19167757fc5bef03c4a4d2b2cc19bd97ec4 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Wed, 15 Jan 2025 22:12:42 +0100 Subject: [PATCH 09/16] Merge with main --- core/vdbe/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index af9f43f78..16182a85f 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -1514,12 +1514,12 @@ impl Program { let reg_values = &state.registers[*start_reg..*start_reg + arg_count]; - let func = match func { + let json_func = match json_func { JsonFunc::JsonArray => json_array, JsonFunc::JsonObject => json_object, _ => unreachable!(), }; - let json_result = func(reg_values); + let json_result = json_func(reg_values); match json_result { Ok(json) => state.registers[*dest] = json, From ac48be4ff1b1791b572bb87dabc8dc9075376caa Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Wed, 15 Jan 2025 22:32:00 +0100 Subject: [PATCH 10/16] chore: remove TODO --- core/json/mod.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index de97c8f78..ad35164b0 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -257,21 +257,22 @@ fn convert_json_to_db_type(extracted: &Val, all_as_db: bool) -> crate::Result` for `Val` here, instead of this? // the same with `convert_json_to_db_type`... +/// Converts a DB value (`OwnedValue`) to a JSON representation (`Val`). +/// Note that when the internal text value is a json, +/// the returned `Val` will be an object. If the internal text value is a regular text, +/// then a string will be returned. This is useful to track if the value came from a json +/// function and therefore we must interpret it as json instead of raw text when working with it. fn convert_db_type_to_json(value: &OwnedValue) -> crate::Result { let val = match value { OwnedValue::Null => Val::Null, OwnedValue::Float(f) => Val::Float(*f), OwnedValue::Integer(i) => Val::Integer(*i), OwnedValue::Text(t) => match t.subtype { - // Convert only to json if the subtype is json (got it from another json function) + // Convert only to json if the subtype is json (if we got it from another json function) TextSubtype::Json => get_json_value(value)?, - TextSubtype::Text => { - let text = t.value.to_string(); - Val::String(text) - } + TextSubtype::Text => Val::String(t.value.to_string()), }, _ => crate::bail_constraint_error!("JSON cannot hold BLOB values"), }; @@ -421,25 +422,21 @@ pub fn json_error_position(json: &OwnedValue) -> crate::Result { } } -// TODO: document this +/// Constructs a JSON object from a list of values that represent key-value pairs. +/// The number of values must be even, and the first value of each pair (which represents the map key) +/// must be a TEXT value. The second value of each pair can be any JSON value (which represents the map value) pub fn json_object(values: &[OwnedValue]) -> crate::Result { let value_map = values .chunks(2) .map(|chunk| match chunk { [key, value] => { let key = match key { - // TODO: is this to_string call ok? // TODO: We can construct the IndexMap from Rc, but we must enable the // serde's `rc` feature so we can serialize Rc OwnedValue::Text(t) => t.value.to_string(), // TODO: I matched sqlite message error here. Is this ok? _ => crate::bail_constraint_error!("labels must be TEXT"), }; - - // TODO: with `convert_db_type_to_json` we convert the value to JSON only if - // it was obtained from another JSON function. - // TODO: the `json_array` function should use something like this. - // That implementation could be a lot simpler with this function let json_val = convert_db_type_to_json(value)?; Ok((key, json_val)) From 8de42faa41d29ad3eb40ce1791d3509729649622 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Wed, 15 Jan 2025 22:34:23 +0100 Subject: [PATCH 11/16] chore: remove TODO --- core/json/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index ad35164b0..455b709e2 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -431,10 +431,7 @@ pub fn json_object(values: &[OwnedValue]) -> crate::Result { .map(|chunk| match chunk { [key, value] => { let key = match key { - // TODO: We can construct the IndexMap from Rc, but we must enable the - // serde's `rc` feature so we can serialize Rc OwnedValue::Text(t) => t.value.to_string(), - // TODO: I matched sqlite message error here. Is this ok? _ => crate::bail_constraint_error!("labels must be TEXT"), }; let json_val = convert_db_type_to_json(value)?; From 71c9a1c9e37bd9d4d39577f37babd9eede7eff80 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Wed, 15 Jan 2025 22:35:56 +0100 Subject: [PATCH 12/16] chore: remove TODO --- core/json/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 455b709e2..91d4de574 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -257,8 +257,6 @@ fn convert_json_to_db_type(extracted: &Val, all_as_db: bool) -> crate::Result` for `Val` here, instead of this? -// the same with `convert_json_to_db_type`... /// Converts a DB value (`OwnedValue`) to a JSON representation (`Val`). /// Note that when the internal text value is a json, /// the returned `Val` will be an object. If the internal text value is a regular text, From f38da70920a893cdee48f7ca4b2f1c623412fad7 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Wed, 15 Jan 2025 22:38:49 +0100 Subject: [PATCH 13/16] chore: remove TODO --- core/json/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 91d4de574..47d736954 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -930,7 +930,7 @@ mod tests { r#"{"parent_key":{"key":"value"}}"# ); } - // TODO: is this behaviour ok? it differs from sqlite + #[test] fn test_json_object_duplicated_keys() { let key = OwnedValue::build_text(Rc::new("key".to_string())); From f6655b38c4c0520c7f3a17185496d5923dbb7e79 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Wed, 15 Jan 2025 22:40:13 +0100 Subject: [PATCH 14/16] chore: remove TODO --- core/json/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 47d736954..6213d7ff7 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -438,7 +438,6 @@ pub fn json_object(values: &[OwnedValue]) -> crate::Result { } _ => crate::bail_constraint_error!("json_object requires an even number of values"), }) - // TODO: collecting into a IndexMap does not allow for repeated keys .collect::, _>>()?; let result = crate::json::to_string(&value_map).unwrap(); From f7ec7bd20bc05f80708a668796f45c53920977db Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Thu, 16 Jan 2025 19:22:39 +0100 Subject: [PATCH 15/16] feat: remove wildcard in convert_db_type_to_json --- core/json/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 6213d7ff7..a279393ca 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -272,7 +272,10 @@ fn convert_db_type_to_json(value: &OwnedValue) -> crate::Result { TextSubtype::Json => get_json_value(value)?, TextSubtype::Text => Val::String(t.value.to_string()), }, - _ => crate::bail_constraint_error!("JSON cannot hold BLOB values"), + OwnedValue::Blob(_) => crate::bail_constraint_error!("JSON cannot hold BLOB values"), + unsupported_value => crate::bail_constraint_error!( + "JSON cannot hold this type of value: {unsupported_value:?}" + ), }; Ok(val) } From b79bc8f8b6fbdecf7ef1164142821feec44fe5a0 Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Sat, 18 Jan 2025 09:39:22 +0100 Subject: [PATCH 16/16] trigger CI