From 70396d74255af47b9108e2da801aca7863828b11 Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 17:15:16 +0200 Subject: [PATCH 01/12] add function definition --- core/function.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/function.rs b/core/function.rs index 1e5386696..ffd84b3f0 100644 --- a/core/function.rs +++ b/core/function.rs @@ -80,6 +80,7 @@ pub enum JsonFunc { JsonType, JsonErrorPosition, JsonValid, + JsonPatch, } #[cfg(feature = "json")] @@ -99,6 +100,7 @@ impl Display for JsonFunc { Self::JsonType => "json_type".to_string(), Self::JsonErrorPosition => "json_error_position".to_string(), Self::JsonValid => "json_valid".to_string(), + Self::JsonPatch => "json_patch".to_string(), } ) } @@ -523,6 +525,8 @@ impl Func { "json_error_position" => Ok(Self::Json(JsonFunc::JsonErrorPosition)), #[cfg(feature = "json")] "json_valid" => Ok(Self::Json(JsonFunc::JsonValid)), + #[cfg(feature = "json")] + "json_patch" => Ok(Self::Json(JsonFunc::JsonPatch)), "unixepoch" => Ok(Self::Scalar(ScalarFunc::UnixEpoch)), "julianday" => Ok(Self::Scalar(ScalarFunc::JulianDay)), "hex" => Ok(Self::Scalar(ScalarFunc::Hex)), From 98be735f5a26ab58195d637c0bb8099b1760b58d Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 19:15:56 +0200 Subject: [PATCH 02/12] add json_patch to expr and vm --- core/translate/expr.rs | 11 +++++++++++ core/vdbe/mod.rs | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 9fdf5f9c2..0050662c0 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -937,6 +937,17 @@ pub fn translate_expr( target_register, func_ctx, ), + JsonFunc::JsonPatch => { + let args = expect_arguments_exact!(args, 2, j); + translate_function( + program, + args, + referenced_tables, + resolver, + target_register, + func_ctx, + ) + } }, Func::Scalar(srf) => { match srf { diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 36cc8e4cb..471aa39cf 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -42,7 +42,8 @@ use crate::vdbe::insn::Insn; use crate::{ function::JsonFunc, json::get_json, json::is_json_valid, 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_object, json::json_type, + json::json_error_position, json::json_extract, json::json_object, json::json_patch, + json::json_type, }; use crate::{resolve_ext_path, Connection, Result, TransactionState, DATABASE_VERSION}; use datetime::{ @@ -1746,6 +1747,13 @@ impl Program { let json_value = &state.registers[*start_reg]; state.registers[*dest] = is_json_valid(json_value)?; } + JsonFunc::JsonPatch => { + assert_eq!(arg_count, 2); + assert!(*start_reg + 1 < state.registers.len()); + let target = &state.registers[*start_reg]; + let patch = &state.registers[*start_reg + 1]; + state.registers[*dest] = json_patch(target, patch)?; + } }, crate::function::Func::Scalar(scalar_func) => match scalar_func { ScalarFunc::Cast => { From 846a73188af3349cf4c087978d4df91e5f2f208a Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 20:00:15 +0200 Subject: [PATCH 03/12] add json_patch implementation --- core/json/json_operations.rs | 103 +++++++++++++++++++++++++++++++++++ core/json/mod.rs | 2 + 2 files changed, 105 insertions(+) create mode 100644 core/json/json_operations.rs diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs new file mode 100644 index 000000000..6a91e157a --- /dev/null +++ b/core/json/json_operations.rs @@ -0,0 +1,103 @@ +use std::collections::VecDeque; + +use crate::types::OwnedValue; + +use super::{convert_json_to_db_type, get_json_value, Val}; + +/// Represents a single patch operation in the merge queue. +/// +/// Used internally by the `merge_patch` function to track the path and value +/// for each pending merge operation. +#[derive(Debug, Clone)] +struct PatchOperation { + path: Vec, + patch: Val, +} + +/// The function follows RFC 7386 JSON Merge Patch semantics: +/// * If the patch is null, the target is replaced with null +/// * If the patch contains a scalar value, the target is replaced with that value +/// * If both target and patch are objects, the patch is recursively applied +/// * null values in the patch result in property removal from the target +pub fn json_patch(target: &OwnedValue, patch: &OwnedValue) -> crate::Result { + match (target, patch) { + (OwnedValue::Blob(_), _) | (_, OwnedValue::Blob(_)) => { + crate::bail_constraint_error!("blob is not supported!"); + } + _ => (), + } + + let mut parsed_target = get_json_value(target)?; + let parsed_patch = get_json_value(patch)?; + + merge_patch(&mut parsed_target, parsed_patch); + + convert_json_to_db_type(&parsed_target, false) +} + +/// # Implementation Notes +/// +/// * Processes patches in breadth-first order +/// * Maintains path information for nested updates +/// * Handles object merging with proper null value semantics +/// * Preserves existing values not mentioned in the patch +/// +/// Following RFC 7386 rules: +/// * null values remove properties +/// * Objects are merged recursively +/// * Non-object values replace the target completely +fn merge_patch(target: &mut Val, patch: Val) { + let mut queue = VecDeque::with_capacity(8); + + queue.push_back(PatchOperation { + path: Vec::new(), + patch, + }); + while let Some(PatchOperation { path, patch }) = queue.pop_front() { + let mut current: &mut Val = target; + + for (depth, key) in path.iter().enumerate() { + if let Val::Object(ref mut obj) = current { + current = &mut obj + .get_mut(*key) + .unwrap_or_else(|| { + panic!("Invalid path at depth {}: key '{}' not found", depth, key) + }) + .1; + } + } + + match (current, patch) { + (curr, Val::Null) => { + *curr = Val::Null; + } + (Val::Object(target_map), Val::Object(patch_map)) => { + for (key, patch_val) in patch_map { + if let Some(pos) = target_map + .iter() + .position(|(target_key, _)| target_key == &key) + { + match patch_val { + Val::Null => { + target_map.remove(pos); + } + val => { + let mut new_path = path.clone(); + new_path.push(pos); + queue.push_back(PatchOperation { + path: new_path, + patch: val, + }); + } + }; + } else { + target_map.push((key, patch_val)); + }; + } + } + (curr, patch_val) => { + *curr = patch_val; + } + } + } +} diff --git a/core/json/mod.rs b/core/json/mod.rs index 8b271c8e1..65e90697f 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -1,5 +1,6 @@ mod de; mod error; +mod json_operations; mod json_path; mod ser; @@ -8,6 +9,7 @@ use std::rc::Rc; pub use crate::json::de::from_str; use crate::json::de::ordered_object; use crate::json::error::Error as JsonError; +pub use crate::json::json_operations::json_patch; use crate::json::json_path::{json_path, JsonPath, PathElement}; pub use crate::json::ser::to_string; use crate::types::{LimboText, OwnedValue, TextSubtype}; From 3f7458faef852ffd6e51008cda000e51874ddf14 Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 20:06:14 +0200 Subject: [PATCH 04/12] add general tests --- testing/json.test | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/testing/json.test b/testing/json.test index 5817a8c55..3b444cfff 100755 --- a/testing/json.test +++ b/testing/json.test @@ -576,3 +576,45 @@ do_execsql_test json_valid_3 { do_execsql_test json_valid_9 { SELECT json_valid(NULL); } {} +do_execsql_test json-patch-basic-1 { + select json_patch('{"a":1}', '{"b":2}'); +} {{{"a":1,"b":2}}} +do_execsql_test json-patch-basic-2 { + select json_patch('{"x":100,"y":200}', '{"z":300}'); +} {{{"x":100,"y":200,"z":300}}} +do_execsql_test json-patch-override-1 { + select json_patch('{"a":1,"b":2}', '{"b":3}'); +} {{{"a":1,"b":3}}} +do_execsql_test json-patch-override-2 { + select json_patch('{"name":"john","age":25}', '{"age":26,"city":"NYC"}'); +} {{{"name":"john","age":26,"city":"NYC"}}} +do_execsql_test json-patch-nested-1 { + select json_patch('{"user":{"name":"john"}}', '{"user":{"age":30}}'); +} {{{"user":{"name":"john","age":30}}}} +do_execsql_test json-patch-nested-2 { + select json_patch('{"settings":{"theme":"dark"}}', '{"settings":{"theme":"light","font":"arial"}}'); +} {{{"settings":{"theme":"light","font":"arial"}}}} +do_execsql_test json-patch-array-1 { + select json_patch('{"arr":[1,2,3]}', '{"arr":[4,5,6]}'); +} {{{"arr":[4,5,6]}}} +do_execsql_test json-patch-array-2 { + select json_patch('{"list":["a","b"]}', '{"list":["c"]}'); +} {{{"list":["c"]}}} +do_execsql_test json-patch-empty-1 { + select json_patch('{}', '{"a":1}'); +} {{{"a":1}}} +do_execsql_test json-patch-empty-2 { + select json_patch('{"a":1}', '{}'); +} {{{"a":1}}} +do_execsql_test json-patch-deep-nested-1 { + select json_patch( + '{"level1":{"level2":{"value":100}}}', + '{"level1":{"level2":{"newValue":200}}}' + ); +} {{{"level1":{"level2":{"value":100,"newValue":200}}}}} +do_execsql_test json-patch-mixed-types-1 { + select json_patch( + '{"str":"hello","num":42,"bool":true}', + '{"arr":[1,2,3],"obj":{"x":1}}' + ); +} {{{"str":"hello","num":42,"bool":true,"arr":[1,2,3],"obj":{"x":1}}}} From 6a605939e692e3b08e7140159d83358da441e8b2 Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 20:41:45 +0200 Subject: [PATCH 05/12] naming refine --- core/json/json_operations.rs | 105 +++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 4 deletions(-) diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs index 6a91e157a..95298febb 100644 --- a/core/json/json_operations.rs +++ b/core/json/json_operations.rs @@ -68,8 +68,8 @@ fn merge_patch(target: &mut Val, patch: Val) { } match (current, patch) { - (curr, Val::Null) => { - *curr = Val::Null; + (current_val, Val::Null) => { + *current_val = Val::Null; } (Val::Object(target_map), Val::Object(patch_map)) => { for (key, patch_val) in patch_map { @@ -95,9 +95,106 @@ fn merge_patch(target: &mut Val, patch: Val) { }; } } - (curr, patch_val) => { - *curr = patch_val; + (current_val, patch_val) => { + *current_val = patch_val; } } } } + +#[cfg(test)] +mod tests { + use std::rc::Rc; + + use crate::types::LimboText; + + use super::*; + + fn create_text(s: &str) -> OwnedValue { + OwnedValue::Text(LimboText::new(Rc::new(s.to_string()))) + } + + fn create_json(s: &str) -> OwnedValue { + OwnedValue::Text(LimboText::json(Rc::new(s.to_string()))) + } + + #[test] + fn test_basic_text_replacement() { + let target = create_text(r#"{"name":"John","age":"30"}"#); + let patch = create_text(r#"{"age":"31"}"#); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!(result, create_json(r#"{"name":"John","age":"31"}"#)); + } + + #[test] + fn test_null_field_removal() { + let target = create_text(r#"{"name":"John","email":"john@example.com"}"#); + let patch = create_text(r#"{"email":null}"#); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!(result, create_json(r#"{"name":"John"}"#)); + } + + #[test] + fn test_nested_object_merge() { + let target = + create_text(r#"{"user":{"name":"John","details":{"age":"30","score":"95.5"}}}"#); + + let patch = create_text(r#"{"user":{"details":{"score":"97.5"}}}"#); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!( + result, + create_json(r#"{"user":{"name":"John","details":{"age":"30","score":"97.5"}}}"#) + ); + } + + #[test] + #[should_panic(expected = "blob is not supported!")] + fn test_blob_not_supported() { + let target = OwnedValue::Blob(Rc::new(vec![1, 2, 3])); + let patch = create_text("{}"); + json_patch(&target, &patch).unwrap(); + } + + #[test] + fn test_deep_null_replacement() { + let target = create_text(r#"{"level1":{"level2":{"keep":"value","remove":"value"}}}"#); + + let patch = create_text(r#"{"level1":{"level2":{"remove":null}}}"#); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!( + result, + create_json(r#"{"level1":{"level2":{"keep":"value"}}}"#) + ); + } + + #[test] + fn test_empty_patch() { + let target = create_json(r#"{"name":"John","age":"30"}"#); + let patch = create_text("{}"); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!(result, target); + } + + #[test] + fn test_add_new_field() { + let target = create_text(r#"{"existing":"value"}"#); + let patch = create_text(r#"{"new":"field"}"#); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!(result, create_json(r#"{"existing":"value","new":"field"}"#)); + } + + #[test] + fn test_complete_object_replacement() { + let target = create_text(r#"{"old":{"nested":"value"}}"#); + let patch = create_text(r#"{"old":"new_value"}"#); + + let result = json_patch(&target, &patch).unwrap(); + assert_eq!(result, create_json(r#"{"old":"new_value"}"#)); + } +} From 8ba43bfc1710bacedf75c6a109ac014c7f9e5c1c Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 20:50:48 +0200 Subject: [PATCH 06/12] add tests for duplicate preservation and first-one-win behavior --- testing/json.test | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/testing/json.test b/testing/json.test index 3b444cfff..78e6171c8 100755 --- a/testing/json.test +++ b/testing/json.test @@ -8,6 +8,10 @@ do_execsql_test json5-ecma-script-1 { } {{{"a":5,"b":6}}} do_execsql_test json5-ecma-script-2 { + select json('{a:5,a:3}') ; +} {{{"a":5,"a":3}}} + +do_execsql_test json5-ecma-script-3 { SELECT json('{ MNO_123$xyz : 789 }'); } {{{"MNO_123$xyz":789}}} @@ -582,6 +586,12 @@ do_execsql_test json-patch-basic-1 { do_execsql_test json-patch-basic-2 { select json_patch('{"x":100,"y":200}', '{"z":300}'); } {{{"x":100,"y":200,"z":300}}} +do_execsql_test json-patch-preserve-duplicates-1 { + select json_patch('{"x":100,"x":200}', '{"z":300}'); +} {{{"x":100,"x":200,"z":300}}} +do_execsql_test json-patch-preserve-duplicates-2 { + select json_patch('{"x":100,"x":200}', '{"x":900}'); +} {{{"x":900,"x":200}}} do_execsql_test json-patch-override-1 { select json_patch('{"a":1,"b":2}', '{"b":3}'); } {{{"a":1,"b":3}}} From f164dbdb1e5fc316e78b5308f908473be37b4103 Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Tue, 28 Jan 2025 21:26:05 +0200 Subject: [PATCH 07/12] fix case where first patched value applied wins --- core/json/json_operations.rs | 8 ++++++-- testing/json.test | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs index 95298febb..735674863 100644 --- a/core/json/json_operations.rs +++ b/core/json/json_operations.rs @@ -1,4 +1,4 @@ -use std::collections::VecDeque; +use std::collections::{HashSet, VecDeque}; use crate::types::OwnedValue; @@ -48,7 +48,7 @@ pub fn json_patch(target: &OwnedValue, patch: &OwnedValue) -> crate::Result::new(); queue.push_back(PatchOperation { path: Vec::new(), patch, @@ -72,7 +72,11 @@ fn merge_patch(target: &mut Val, patch: Val) { *current_val = Val::Null; } (Val::Object(target_map), Val::Object(patch_map)) => { + applied_keys.clear(); for (key, patch_val) in patch_map { + if applied_keys.insert(key.clone()) == false { + continue; + } if let Some(pos) = target_map .iter() .position(|(target_key, _)| target_key == &key) diff --git a/testing/json.test b/testing/json.test index 78e6171c8..d156572e0 100755 --- a/testing/json.test +++ b/testing/json.test @@ -592,6 +592,9 @@ do_execsql_test json-patch-preserve-duplicates-1 { do_execsql_test json-patch-preserve-duplicates-2 { select json_patch('{"x":100,"x":200}', '{"x":900}'); } {{{"x":900,"x":200}}} +do_execsql_test json-patch-first-update-wins { + select json_patch('{"x":100,"c":200}', '{"x":900, "x":null}'); +} {{{"x":900,"c":200}}} do_execsql_test json-patch-override-1 { select json_patch('{"a":1,"b":2}', '{"b":3}'); } {{{"a":1,"b":3}}} From 2407a29e9022fce038b4b702090c55ece0a476cf Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Wed, 29 Jan 2025 03:13:31 +0200 Subject: [PATCH 08/12] refactored into patcher struct and made code more modular and readable --- core/json/json_operations.rs | 147 ++++++++++++++++++++++------------- 1 file changed, 92 insertions(+), 55 deletions(-) diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs index 735674863..7d7679752 100644 --- a/core/json/json_operations.rs +++ b/core/json/json_operations.rs @@ -29,81 +29,118 @@ pub fn json_patch(target: &OwnedValue, patch: &OwnedValue) -> crate::Result::new(); - queue.push_back(PatchOperation { - path: Vec::new(), - patch, - }); - while let Some(PatchOperation { path, patch }) = queue.pop_front() { - let mut current: &mut Val = target; +struct Patcher { + queue: VecDeque, + applied_keys: HashSet, +} - for (depth, key) in path.iter().enumerate() { +impl Patcher { + fn new(queue_capacity: usize) -> Self { + Self { + queue: VecDeque::with_capacity(queue_capacity), + applied_keys: HashSet::new(), + } + } + + fn apply_patch(&mut self, target: &mut Val, patch: Val) { + self.queue.push_back(PatchOperation { + path: Vec::new(), + patch, + }); + + while let Some(op) = self.queue.pop_front() { + let current = self.navigate_to_target(target, &op.path); + self.apply_operation(current, op.patch, &op.path); + } + } + + fn navigate_to_target<'a>(&self, target: &'a mut Val, path: &Vec) -> &'a mut Val { + let mut current = target; + for (depth, &key) in path.iter().enumerate() { if let Val::Object(ref mut obj) = current { current = &mut obj - .get_mut(*key) + .get_mut(key) .unwrap_or_else(|| { panic!("Invalid path at depth {}: key '{}' not found", depth, key) }) .1; } } + current + } + fn apply_operation(&mut self, current: &mut Val, patch: Val, path: &Vec) { match (current, patch) { - (current_val, Val::Null) => { - *current_val = Val::Null; - } + (current_val, Val::Null) => *current_val = Val::Null, (Val::Object(target_map), Val::Object(patch_map)) => { - applied_keys.clear(); - for (key, patch_val) in patch_map { - if applied_keys.insert(key.clone()) == false { - continue; - } - if let Some(pos) = target_map - .iter() - .position(|(target_key, _)| target_key == &key) - { - match patch_val { - Val::Null => { - target_map.remove(pos); - } - val => { - let mut new_path = path.clone(); - new_path.push(pos); - queue.push_back(PatchOperation { - path: new_path, - patch: val, - }); - } - }; - } else { - target_map.push((key, patch_val)); - }; + self.merge_objects(target_map, patch_map, path); + } + (current_val, patch_val) => *current_val = patch_val, + } + } + + fn merge_objects( + &mut self, + target_map: &mut Vec<(String, Val)>, + patch_map: Vec<(String, Val)>, + path: &Vec, + ) { + self.applied_keys.clear(); + let original_keys: Vec = target_map.iter().map(|(key, _)| key.clone()).collect(); + + for (key, patch_val) in patch_map { + self.process_key_value(target_map, &original_keys, key, patch_val, path); + } + } + + fn process_key_value( + &mut self, + target_map: &mut Vec<(String, Val)>, + original_keys: &[String], + key: String, + patch_val: Val, + path: &Vec, + ) { + if !original_keys.contains(&key) { + target_map.push((key, patch_val)); + return; + } + + if let Some(pos) = target_map + .iter() + .position(|(target_key, _)| target_key == &key) + { + if !self.applied_keys.insert(key) { + return; + } + + match patch_val { + Val::Null => { + target_map.remove(pos); + } + val => { + self.queue_nested_patch(pos, val, path); } } - (current_val, patch_val) => { - *current_val = patch_val; - } + } else { + target_map.push((key, patch_val)); } } + + fn queue_nested_patch(&mut self, pos: usize, val: Val, path: &Vec) { + let mut new_path = path.clone(); + new_path.push(pos); + self.queue.push_back(PatchOperation { + path: new_path, + patch: val, + }); + } } #[cfg(test)] From 0714ab64afd5eeb734159999f8b2e1874a159ff2 Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Wed, 29 Jan 2025 03:14:53 +0200 Subject: [PATCH 09/12] add tests for tricky edge cases --- testing/json.test | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testing/json.test b/testing/json.test index d156572e0..850e0351a 100755 --- a/testing/json.test +++ b/testing/json.test @@ -631,3 +631,15 @@ do_execsql_test json-patch-mixed-types-1 { '{"arr":[1,2,3],"obj":{"x":1}}' ); } {{{"str":"hello","num":42,"bool":true,"arr":[1,2,3],"obj":{"x":1}}}} +do_execsql_test json-patch-add-all-dup-keys-from-patch { + select json_patch( + '{"x":100,"x":200}', + '{"z":{}, "z":5, "z":100}' + ); +} {{{"x":100,"x":200,"z":{},"z":5,"z":100}}} +do_execsql_test json-patch-first-occurance-patch { + select json_patch( + '{"x":100,"x":200}', + '{"x":{}, "x":5, "x":100}' + ); +} {{{"x":{},"x":200}}} From 166d0a7165ef5c3c83274f130d93d615c99301df Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Wed, 29 Jan 2025 16:39:38 +0200 Subject: [PATCH 10/12] add more tests and fixed test for latest sqlite v --- testing/json.test | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/testing/json.test b/testing/json.test index 850e0351a..751d26bc7 100755 --- a/testing/json.test +++ b/testing/json.test @@ -592,9 +592,9 @@ do_execsql_test json-patch-preserve-duplicates-1 { do_execsql_test json-patch-preserve-duplicates-2 { select json_patch('{"x":100,"x":200}', '{"x":900}'); } {{{"x":900,"x":200}}} -do_execsql_test json-patch-first-update-wins { +do_execsql_test json-patch-last-update-wins { select json_patch('{"x":100,"c":200}', '{"x":900, "x":null}'); -} {{{"x":900,"c":200}}} +} {{{"c":200}}} do_execsql_test json-patch-override-1 { select json_patch('{"a":1,"b":2}', '{"b":3}'); } {{{"a":1,"b":3}}} @@ -636,10 +636,43 @@ do_execsql_test json-patch-add-all-dup-keys-from-patch { '{"x":100,"x":200}', '{"z":{}, "z":5, "z":100}' ); -} {{{"x":100,"x":200,"z":{},"z":5,"z":100}}} +} {{{"x":100,"x":200,"z":100}}} do_execsql_test json-patch-first-occurance-patch { + select json_patch('{"x":100,"x":200}','{"x":{}, "x":5, "x":100}'); +} {{{"x":100,"x":200}}} +do_execsql_test json-patch-complex-nested-dup-keys { select json_patch( - '{"x":100,"x":200}', - '{"x":{}, "x":5, "x":100}' + '{"a":{"x":1,"x":2},"a":{"y":3},"b":[{"z":4,"z":5}]}', + '{"a":{"w":6},"b":[{"z":7,"z":8}],"b":{"z":9}}' ); -} {{{"x":{},"x":200}}} +} {{{"a":{"x":1,"x":2,"w":6},"a":{"y":3},"b":{"z":9}}}} +do_execsql_test json-patch-unicode-dup-keys { + select json_patch( + '{"🔑":1,"🔑":2}', + '{"🗝️":3,"🗝️":4}' + ); +} {{{"🔑":1,"🔑":2,"🗝️":4}}} +do_execsql_test json-patch-empty-string-dup-keys { + select json_patch( + '{"":1,"":2}', + '{"":3,"":4}' + ); +} {{{"":4,"":2}}} +do_execsql_test json-patch-multiple-types-dup-keys { + select json_patch( + '{"x":100,"x":"str","x":true,"x":null}', + '{"y":1,"y":{},"y":[],"y":false}' + ); +} {{{"x":100,"x":"str","x":true,"x":null,"y":false}}} +do_execsql_test json-patch-deep-nested-dup-keys { + select json_patch( + '{"a":{"b":{"c":1}},"a":{"b":{"c":2}},"a":{"b":{"d":3}}}', + '{"x":{"y":{"z":4}},"x":{"y":{"z":5}}}' + ); +} {{{"a":{"b":{"c":1}},"a":{"b":{"c":2}},"a":{"b":{"d":3}},"x":{"y":{"z":5}}}}} +do_execsql_test json-patch-abomination { + select json_patch( + '{"a":{"b":{"x":1,"x":2,"y":{"z":3,"z":{"w":4}}},"b":[{"c":5,"c":6},{"d":{"e":7,"e":null}}],"f":{"g":[1,2,3],"g":{"h":8,"h":[4,5,6]}},"i":{"j":true,"j":{"k":false,"k":{"l":null,"l":"string"}}},"m":{"n":{"o":{"p":9,"p":{"q":10}},"o":{"r":11}}},"m":[{"s":{"t":12}},{"s":{"t":13,"t":{"u":14}}}]},"a":{"v":{"w":{"x":{"y":{"z":15}}}},"v":{"w":{"x":16,"x":{"y":17}}},"aa":[{"bb":{"cc":18,"cc":{"dd":19}}},{"bb":{"cc":{"dd":20},"cc":21}}]}}', + '{"a":{"b":{"x":{"new":"value"},"y":null},"b":{"c":{"updated":true},"d":{"e":{"replaced":100}}},"f":{"g":{"h":{"nested":"deep"}}},"i":{"j":{"k":{"l":{"modified":false}}}},"m":{"n":{"o":{"p":{"q":{"extra":"level"}}}},"s":null},"aa":[{"bb":{"cc":{"dd":{"ee":"new"}}}},{"bb":{"cc":{"dd":{"ff":"value"}}}}],"v":{"w":{"x":{"y":{"z":{"final":"update"}}}}}},"newTop":{"level":{"key":{"with":{"deep":{"nesting":true}}},"key":[{"array":{"in":{"deep":{"structure":null}}}}]}}}' + ); +} {{{"a":{"b":{"x":{"new":"value"},"x":2,"c":{"updated":true},"d":{"e":{"replaced":100}}},"b":[{"c":5,"c":6},{"d":{"e":7,"e":null}}],"f":{"g":{"h":{"nested":"deep"}},"g":{"h":8,"h":[4,5,6]}},"i":{"j":{"k":{"l":{"modified":false}}},"j":{"k":false,"k":{"l":null,"l":"string"}}},"m":{"n":{"o":{"p":{"q":{"extra":"level"}},"p":{"q":10}},"o":{"r":11}}},"m":[{"s":{"t":12}},{"s":{"t":13,"t":{"u":14}}}],"aa":[{"bb":{"cc":{"dd":{"ee":"new"}}}},{"bb":{"cc":{"dd":{"ff":"value"}}}}],"v":{"w":{"x":{"y":{"z":{"final":"update"}}}}}},"a":{"v":{"w":{"x":{"y":{"z":15}}}},"v":{"w":{"x":16,"x":{"y":17}}},"aa":[{"bb":{"cc":18,"cc":{"dd":19}}},{"bb":{"cc":{"dd":20},"cc":21}}]},"newTop":{"level":{"key":[{"array":{"in":{"deep":{"structure":null}}}}]}}}}} From 4e7c4e7cedb11bacff8ef1766d354360627fbbd9 Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Wed, 29 Jan 2025 16:47:03 +0200 Subject: [PATCH 11/12] fixes to pass tests --- core/json/json_operations.rs | 93 +++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs index 7d7679752..06d81c318 100644 --- a/core/json/json_operations.rs +++ b/core/json/json_operations.rs @@ -1,4 +1,4 @@ -use std::collections::{HashSet, VecDeque}; +use std::collections::VecDeque; use crate::types::OwnedValue; @@ -10,7 +10,8 @@ use super::{convert_json_to_db_type, get_json_value, Val}; /// for each pending merge operation. #[derive(Debug, Clone)] struct PatchOperation { - path: Vec, + path_start: usize, + path_len: usize, patch: Val, } @@ -37,49 +38,63 @@ pub fn json_patch(target: &OwnedValue, patch: &OwnedValue) -> crate::Result, - applied_keys: HashSet, + path_storage: Vec, } impl Patcher { fn new(queue_capacity: usize) -> Self { Self { queue: VecDeque::with_capacity(queue_capacity), - applied_keys: HashSet::new(), + path_storage: Vec::with_capacity(256), } } fn apply_patch(&mut self, target: &mut Val, patch: Val) { self.queue.push_back(PatchOperation { - path: Vec::new(), + path_start: 0, + path_len: 0, patch, }); while let Some(op) = self.queue.pop_front() { - let current = self.navigate_to_target(target, &op.path); - self.apply_operation(current, op.patch, &op.path); + if let Some(current) = self.navigate_to_target(target, op.path_start, op.path_len) { + self.apply_operation(current, op); + } else { + continue; + } } } - fn navigate_to_target<'a>(&self, target: &'a mut Val, path: &Vec) -> &'a mut Val { + fn navigate_to_target<'a>( + &self, + target: &'a mut Val, + path_start: usize, + path_len: usize, + ) -> Option<&'a mut Val> { let mut current = target; - for (depth, &key) in path.iter().enumerate() { + for i in 0..path_len { + let key = self.path_storage[path_start + i]; if let Val::Object(ref mut obj) = current { current = &mut obj .get_mut(key) .unwrap_or_else(|| { - panic!("Invalid path at depth {}: key '{}' not found", depth, key) + panic!("Invalid path at depth {}: key '{}' not found", i, key) }) .1; + } else { + return None; } } - current + Some(current) } - fn apply_operation(&mut self, current: &mut Val, patch: Val, path: &Vec) { - match (current, patch) { - (current_val, Val::Null) => *current_val = Val::Null, + fn apply_operation(&mut self, current: &mut Val, operation: PatchOperation) { + let path_start = operation.path_start; + let path_len = operation.path_len; + match (current, operation.patch) { + (current_val, Val::Null) => *current_val = Val::Removed, (Val::Object(target_map), Val::Object(patch_map)) => { - self.merge_objects(target_map, patch_map, path); + self.merge_objects(target_map, patch_map, path_start, path_len); } (current_val, patch_val) => *current_val = patch_val, } @@ -89,55 +104,43 @@ impl Patcher { &mut self, target_map: &mut Vec<(String, Val)>, patch_map: Vec<(String, Val)>, - path: &Vec, + path_start: usize, + path_len: usize, ) { - self.applied_keys.clear(); - let original_keys: Vec = target_map.iter().map(|(key, _)| key.clone()).collect(); - for (key, patch_val) in patch_map { - self.process_key_value(target_map, &original_keys, key, patch_val, path); + self.process_key_value(target_map, key, patch_val, path_start, path_len); } } fn process_key_value( &mut self, target_map: &mut Vec<(String, Val)>, - original_keys: &[String], key: String, patch_val: Val, - path: &Vec, + path_start: usize, + path_len: usize, ) { - if !original_keys.contains(&key) { - target_map.push((key, patch_val)); - return; - } - if let Some(pos) = target_map .iter() .position(|(target_key, _)| target_key == &key) { - if !self.applied_keys.insert(key) { - return; - } - - match patch_val { - Val::Null => { - target_map.remove(pos); - } - val => { - self.queue_nested_patch(pos, val, path); - } - } - } else { - target_map.push((key, patch_val)); + self.queue_nested_patch(pos, patch_val, path_start, path_len); + } else if !matches!(patch_val, Val::Null) { + target_map.push((key, Val::Object(vec![]))); + self.queue_nested_patch(target_map.len() - 1, patch_val, path_start, path_len) } } - fn queue_nested_patch(&mut self, pos: usize, val: Val, path: &Vec) { - let mut new_path = path.clone(); - new_path.push(pos); + fn queue_nested_patch(&mut self, pos: usize, val: Val, path_start: usize, path_len: usize) { + let new_path_start = self.path_storage.len(); + let new_path_len = path_len + 1; + for i in 0..path_len { + self.path_storage.push(self.path_storage[path_start + i]); + } + self.path_storage.push(pos); self.queue.push_back(PatchOperation { - path: new_path, + path_start: new_path_start, + path_len: new_path_len, patch: val, }); } From 0048f77b45894e406a3ca44040fd0436b42e163c Mon Sep 17 00:00:00 2001 From: Ihor Andrianov Date: Wed, 29 Jan 2025 17:52:54 +0200 Subject: [PATCH 12/12] add unit tests --- core/json/json_operations.rs | 229 ++++++++++++++++++++++++++++++++++- 1 file changed, 226 insertions(+), 3 deletions(-) diff --git a/core/json/json_operations.rs b/core/json/json_operations.rs index 06d81c318..185aba00c 100644 --- a/core/json/json_operations.rs +++ b/core/json/json_operations.rs @@ -30,18 +30,19 @@ pub fn json_patch(target: &OwnedValue, patch: &OwnedValue) -> crate::Result, path_storage: Vec, } -impl Patcher { +impl JsonPatcher { fn new(queue_capacity: usize) -> Self { Self { queue: VecDeque::with_capacity(queue_capacity), @@ -162,6 +163,228 @@ mod tests { OwnedValue::Text(LimboText::json(Rc::new(s.to_string()))) } + #[test] + fn test_new_patcher() { + let patcher = JsonPatcher::new(10); + assert_eq!(patcher.queue.capacity(), 10); + assert_eq!(patcher.path_storage.capacity(), 256); + assert!(patcher.queue.is_empty()); + assert!(patcher.path_storage.is_empty()); + } + + #[test] + fn test_simple_value_replacement() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![("key1".to_string(), Val::String("old".to_string()))]); + let patch = Val::Object(vec![("key1".to_string(), Val::String("new".to_string()))]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + assert_eq!(map[0].1, Val::String("new".to_string())); + } else { + panic!("Expected object"); + } + } + + #[test] + fn test_nested_object_patch() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![( + "a".to_string(), + Val::Object(vec![("b".to_string(), Val::String("old".to_string()))]), + )]); + let patch = Val::Object(vec![( + "a".to_string(), + Val::Object(vec![("b".to_string(), Val::String("new".to_string()))]), + )]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + if let Val::Object(nested) = &map[0].1 { + assert_eq!(nested[0].1, Val::String("new".to_string())); + } else { + panic!("Expected nested object"); + } + } + } + + #[test] + fn test_null_removal() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![ + ("keep".to_string(), Val::String("value".to_string())), + ("remove".to_string(), Val::String("value".to_string())), + ]); + let patch = Val::Object(vec![("remove".to_string(), Val::Null)]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + assert_eq!(map[0].1, Val::String("value".to_string())); + assert_eq!(map[1].1, Val::Removed); + } + } + + #[test] + fn test_duplicate_keys_first_occurrence() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![ + ("key".to_string(), Val::String("first".to_string())), + ("key".to_string(), Val::String("second".to_string())), + ("key".to_string(), Val::String("third".to_string())), + ]); + let patch = Val::Object(vec![( + "key".to_string(), + Val::String("modified".to_string()), + )]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + assert_eq!(map[0].1, Val::String("modified".to_string())); + assert_eq!(map[1].1, Val::String("second".to_string())); + assert_eq!(map[2].1, Val::String("third".to_string())); + } + } + + #[test] + fn test_add_new_key() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![( + "existing".to_string(), + Val::String("value".to_string()), + )]); + let patch = Val::Object(vec![("new".to_string(), Val::String("value".to_string()))]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + assert_eq!(map.len(), 2); + assert_eq!(map[1].0, "new"); + assert_eq!(map[1].1, Val::String("value".to_string())); + } + } + + #[test] + fn test_deep_nested_patch() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![( + "level1".to_string(), + Val::Object(vec![( + "level2".to_string(), + Val::Object(vec![("level3".to_string(), Val::String("old".to_string()))]), + )]), + )]); + let patch = Val::Object(vec![( + "level1".to_string(), + Val::Object(vec![( + "level2".to_string(), + Val::Object(vec![("level3".to_string(), Val::String("new".to_string()))]), + )]), + )]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(l1) = target { + if let Val::Object(l2) = &l1[0].1 { + if let Val::Object(l3) = &l2[0].1 { + assert_eq!(l3[0].1, Val::String("new".to_string())); + } + } + } + } + + #[test] + fn test_null_patch_on_nonexistent_key() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![( + "existing".to_string(), + Val::String("value".to_string()), + )]); + let patch = Val::Object(vec![("nonexistent".to_string(), Val::Null)]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + assert_eq!(map.len(), 1); // Should not add new key for null patch + assert_eq!(map[0].0, "existing"); + } + } + + #[test] + fn test_nested_duplicate_keys() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![( + "outer".to_string(), + Val::Object(vec![ + ("inner".to_string(), Val::String("first".to_string())), + ("inner".to_string(), Val::String("second".to_string())), + ]), + )]); + let patch = Val::Object(vec![( + "outer".to_string(), + Val::Object(vec![( + "inner".to_string(), + Val::String("modified".to_string()), + )]), + )]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(outer) = target { + if let Val::Object(inner) = &outer[0].1 { + assert_eq!(inner[0].1, Val::String("modified".to_string())); + assert_eq!(inner[1].1, Val::String("second".to_string())); + } + } + } + + #[test] + #[should_panic(expected = "Invalid path")] + fn test_invalid_path_navigation() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![("a".to_string(), Val::Object(vec![]))]); + patcher.path_storage.push(0); + patcher.path_storage.push(999); // Invalid index + + patcher.navigate_to_target(&mut target, 0, 2); + } + + #[test] + fn test_merge_empty_objects() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![]); + let patch = Val::Object(vec![]); + + patcher.apply_patch(&mut target, patch); + + if let Val::Object(map) = target { + assert!(map.is_empty()); + } + } + + #[test] + fn test_path_storage_growth() { + let mut patcher = JsonPatcher::new(10); + let mut target = Val::Object(vec![( + "a".to_string(), + Val::Object(vec![("b".to_string(), Val::Object(vec![]))]), + )]); + let patch = Val::Object(vec![( + "a".to_string(), + Val::Object(vec![("b".to_string(), Val::String("value".to_string()))]), + )]); + + patcher.apply_patch(&mut target, patch); + + // Path storage should contain [0, 0] for accessing a.b + assert_eq!(patcher.path_storage.len(), 3); + assert_eq!(patcher.path_storage[0], 0); + assert_eq!(patcher.path_storage[1], 0); + } + #[test] fn test_basic_text_replacement() { let target = create_text(r#"{"name":"John","age":"30"}"#);