Merge 'json_remove() function implementation' from Ihor Andrianov

Uses already implemented json path parser so shares limitations with
json_extract()

Closes #828

COMPAT.md

@@ -368,7 +368,7 @@ Modifiers:
 | json_patch(json1,json2)            | Yes     |                                                                                                                                              |
 | jsonb_patch(json1,json2)           |         |                                                                                                                                              |
 | json_pretty(json)                  |         |                                                                                                                                              |
-| json_remove(json,path,...)         |         |                                                                                                                                              |
+| json_remove(json,path,...)         | Partial | Uses same json path parser as json_extract so shares same limitations.                                                                       |
 | jsonb_remove(json,path,...)        |         |                                                                                                                                              |
 | json_replace(json,path,value,...)  |         |                                                                                                                                              |
 | jsonb_replace(json,path,value,...) |         |                                                                                                                                              |
@@ -596,7 +596,7 @@ Limbo has in-tree extensions.
 
 UUID's in Limbo are `blobs` by default.
 
-| Function              | Status | Comment                                                       | 
+| Function              | Status | Comment                                                       |
 |-----------------------|--------|---------------------------------------------------------------|
 | uuid4()               | Yes    | UUID version 4                                                |
 | uuid4_str()           | Yes    | UUID v4 string alias `gen_random_uuid()` for PG compatibility |
@@ -609,7 +609,7 @@ UUID's in Limbo are `blobs` by default.
 
 The `regexp` extension is compatible with [sqlean-regexp](https://github.com/nalgeon/sqlean/blob/main/docs/regexp.md).
 
-| Function                                       | Status | Comment | 
+| Function                                       | Status | Comment |
 |------------------------------------------------|--------|---------|
 | regexp(pattern, source)                        | Yes    |         |
 | regexp_like(source, pattern)                   | Yes    |         |
@@ -621,7 +621,7 @@ The `regexp` extension is compatible with [sqlean-regexp](https://github.com/nal
 
 The `vector` extension is compatible with libSQL native vector search.
 
-| Function                                       | Status | Comment | 
+| Function                                       | Status | Comment |
 |------------------------------------------------|--------|---------|
 | vector(x)                                      | Yes    |         |
 | vector32(x)                                    | Yes    |         |

core/function.rs

@@ -81,6 +81,7 @@ pub enum JsonFunc {
     JsonErrorPosition,
     JsonValid,
     JsonPatch,
+    JsonRemove,
 }
 
 #[cfg(feature = "json")]
@@ -101,6 +102,7 @@ impl Display for JsonFunc {
                 Self::JsonErrorPosition => "json_error_position".to_string(),
                 Self::JsonValid => "json_valid".to_string(),
                 Self::JsonPatch => "json_patch".to_string(),
+                Self::JsonRemove => "json_remove".to_string(),
             }
         )
     }
@@ -530,6 +532,8 @@ impl Func {
             "json_valid" => Ok(Self::Json(JsonFunc::JsonValid)),
             #[cfg(feature = "json")]
             "json_patch" => Ok(Self::Json(JsonFunc::JsonPatch)),
+            #[cfg(feature = "json")]
+            "json_remove" => Ok(Self::Json(JsonFunc::JsonRemove)),
             "unixepoch" => Ok(Self::Scalar(ScalarFunc::UnixEpoch)),
             "julianday" => Ok(Self::Scalar(ScalarFunc::JulianDay)),
             "hex" => Ok(Self::Scalar(ScalarFunc::Hex)),

core/json/json_operations.rs

@@ -1,8 +1,11 @@
 use std::collections::VecDeque;
 
-use crate::types::OwnedValue;
+use crate::{
+    json::{mutate_json_by_path, Target},
+    types::OwnedValue,
+};
 
-use super::{convert_json_to_db_type, get_json_value, Val};
+use super::{convert_json_to_db_type, get_json_value, json_path::json_path, Val};
 
 /// Represents a single patch operation in the merge queue.
 ///
@@ -147,6 +150,40 @@ impl JsonPatcher {
     }
 }
 
+pub fn json_remove(args: &[OwnedValue]) -> crate::Result<OwnedValue> {
+    if args.is_empty() {
+        return Ok(OwnedValue::Null);
+    }
+
+    let mut parsed_target = get_json_value(&args[0])?;
+    if args.len() == 1 {
+        return Ok(args[0].clone());
+    }
+
+    let paths: Result<Vec<_>, _> = args[1..]
+        .iter()
+        .map(|path| {
+            if let OwnedValue::Text(path) = path {
+                json_path(&path.value)
+            } else {
+                crate::bail_constraint_error!("bad JSON path: {:?}", path.to_string())
+            }
+        })
+        .collect();
+    let paths = paths?;
+
+    for path in paths {
+        mutate_json_by_path(&mut parsed_target, path, |val| match val {
+            Target::Array(arr, index) => {
+                arr.remove(index);
+            }
+            Target::Value(val) => *val = Val::Removed,
+        });
+    }
+
+    convert_json_to_db_type(&parsed_target, false)
+}
+
 #[cfg(test)]
 mod tests {
     use std::rc::Rc;
@@ -464,4 +501,94 @@ mod tests {
         let result = json_patch(&target, &patch).unwrap();
         assert_eq!(result, create_json(r#"{"old":"new_value"}"#));
     }
+
+    #[test]
+    fn test_json_remove_empty_args() {
+        let args = vec![];
+        assert_eq!(json_remove(&args).unwrap(), OwnedValue::Null);
+    }
+
+    #[test]
+    fn test_json_remove_array_element() {
+        let args = vec![create_json(r#"[1,2,3,4,5]"#), create_text("$[2]")];
+
+        let result = json_remove(&args).unwrap();
+        match result {
+            OwnedValue::Text(t) => assert_eq!(t.value.as_str(), "[1,2,4,5]"),
+            _ => panic!("Expected Text value"),
+        }
+    }
+
+    #[test]
+    fn test_json_remove_multiple_paths() {
+        let args = vec![
+            create_json(r#"{"a": 1, "b": 2, "c": 3}"#),
+            create_text("$.a"),
+            create_text("$.c"),
+        ];
+
+        let result = json_remove(&args).unwrap();
+        match result {
+            OwnedValue::Text(t) => assert_eq!(t.value.as_str(), r#"{"b":2}"#),
+            _ => panic!("Expected Text value"),
+        }
+    }
+
+    #[test]
+    fn test_json_remove_nested_paths() {
+        let args = vec![
+            create_json(r#"{"a": {"b": {"c": 1, "d": 2}}}"#),
+            create_text("$.a.b.c"),
+        ];
+
+        let result = json_remove(&args).unwrap();
+        match result {
+            OwnedValue::Text(t) => assert_eq!(t.value.as_str(), r#"{"a":{"b":{"d":2}}}"#),
+            _ => panic!("Expected Text value"),
+        }
+    }
+
+    #[test]
+    fn test_json_remove_duplicate_keys() {
+        let args = vec![
+            create_json(r#"{"a": 1, "a": 2, "a": 3}"#),
+            create_text("$.a"),
+        ];
+
+        let result = json_remove(&args).unwrap();
+        match result {
+            OwnedValue::Text(t) => assert_eq!(t.value.as_str(), r#"{"a":2,"a":3}"#),
+            _ => panic!("Expected Text value"),
+        }
+    }
+
+    #[test]
+    fn test_json_remove_invalid_path() {
+        let args = vec![
+            create_json(r#"{"a": 1}"#),
+            OwnedValue::Integer(42), // Invalid path type
+        ];
+
+        assert!(json_remove(&args).is_err());
+    }
+
+    #[test]
+    fn test_json_remove_complex_case() {
+        let args = vec![
+            create_json(r#"{"a":[1,2,3],"b":{"x":1,"x":2},"c":[{"y":1},{"y":2}]}"#),
+            create_text("$.a[1]"),
+            create_text("$.b.x"),
+            create_text("$.c[0].y"),
+        ];
+
+        let result = json_remove(&args).unwrap();
+        match result {
+            OwnedValue::Text(t) => {
+                let value = t.value.as_str();
+                assert!(value.contains(r#"[1,3]"#));
+                assert!(value.contains(r#"{"x":2}"#));
+            }
+            _ => panic!("Expected Text value"),
+        }
+    }
 }

core/json/json_path.pest

@@ -4,5 +4,5 @@ array_locator = ${ "[" ~ negative_index_indicator? ~ array_offset ~ "]" }
 relaxed_array_locator = ${ negative_index_indicator? ~ array_offset }
 
 root = ${ "$" }
-json_path_key = ${ identifier | string }
+json_path_key = ${ identifier | string | ASCII_DIGIT+ }
 path = ${ SOI ~ root ~ (array_locator | "." ~ json_path_key)* ~ EOI }

core/json/mod.rs

@@ -9,7 +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;
+pub use crate::json::json_operations::{json_patch, json_remove};
 use crate::json::json_path::{json_path, JsonPath, PathElement};
 pub use crate::json::ser::to_string;
 use crate::types::{LimboText, OwnedValue, TextSubtype};
@@ -241,6 +241,7 @@ pub fn json_extract(value: &OwnedValue, paths: &[OwnedValue]) -> crate::Result<O
 /// *all_as_db* - if true, objects and arrays will be returned as pure TEXT without the JSON subtype
 fn convert_json_to_db_type(extracted: &Val, all_as_db: bool) -> crate::Result<OwnedValue> {
     match extracted {
+        Val::Removed => Ok(OwnedValue::Null),
         Val::Null => Ok(OwnedValue::Null),
         Val::Float(f) => Ok(OwnedValue::Float(*f)),
         Val::Integer(i) => Ok(OwnedValue::Integer(*i)),
@@ -404,6 +405,64 @@ fn json_extract_single<'a>(
     Ok(Some(current_element))
 }
 
+enum Target<'a> {
+    Array(&'a mut Vec<Val>, usize),
+    Value(&'a mut Val),
+}
+
+fn mutate_json_by_path<F, R>(json: &mut Val, path: JsonPath, closure: F) -> Option<R>
+where
+    F: FnMut(Target) -> R,
+{
+    find_target(json, &path).map(closure)
+}
+
+fn find_target<'a>(json: &'a mut Val, path: &JsonPath) -> Option<Target<'a>> {
+    let mut current = json;
+    for (i, key) in path.elements.iter().enumerate() {
+        let is_last = i == path.elements.len() - 1;
+        match key {
+            PathElement::Root() => continue,
+            PathElement::ArrayLocator(index) => match current {
+                Val::Array(arr) => {
+                    if let Some(index) = match index {
+                        i if *i < 0 => arr.len().checked_sub(i.unsigned_abs() as usize),
+                        i => ((*i as usize) < arr.len()).then_some(*i as usize),
+                    } {
+                        if is_last {
+                            return Some(Target::Array(arr, index));
+                        } else {
+                            current = &mut arr[index];
+                        }
+                    } else {
+                        return None;
+                    }
+                }
+                _ => {
+                    return None;
+                }
+            },
+            PathElement::Key(key) => match current {
+                Val::Object(obj) => {
+                    if let Some(pos) = &obj
+                        .iter()
+                        .position(|(k, v)| k == key && !matches!(v, Val::Removed))
+                    {
+                        let val = &mut obj[*pos].1;
+                        current = val;
+                    } else {
+                        return None;
+                    }
+                }
+                _ => {
+                    return None;
+                }
+            },
+        }
+    }
+    Some(Target::Value(current))
+}
+
 pub fn json_error_position(json: &OwnedValue) -> crate::Result<OwnedValue> {
     match json {
         OwnedValue::Text(t) => match from_str::<Val>(&t.value) {
@@ -454,6 +513,21 @@ pub fn json_object(values: &[OwnedValue]) -> crate::Result<OwnedValue> {
     Ok(OwnedValue::Text(LimboText::json(Rc::new(result))))
 }
 
+pub fn is_json_valid(json_value: &OwnedValue) -> crate::Result<OwnedValue> {
+    match json_value {
+        OwnedValue::Text(ref t) => match from_str::<Val>(&t.value) {
+            Ok(_) => Ok(OwnedValue::Integer(1)),
+            Err(_) => Ok(OwnedValue::Integer(0)),
+        },
+        OwnedValue::Blob(b) => match jsonb::from_slice(b) {
+            Ok(_) => Ok(OwnedValue::Integer(1)),
+            Err(_) => Ok(OwnedValue::Integer(0)),
+        },
+        OwnedValue::Null => Ok(OwnedValue::Null),
+        _ => Ok(OwnedValue::Integer(1)),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -989,18 +1063,98 @@ mod tests {
                 .contains("json_object requires an even number of values")),
         }
     }
-}
-pub fn is_json_valid(json_value: &OwnedValue) -> crate::Result<OwnedValue> {
-    match json_value {
-        OwnedValue::Text(ref t) => match from_str::<Val>(&t.value) {
-            Ok(_) => Ok(OwnedValue::Integer(1)),
-            Err(_) => Ok(OwnedValue::Integer(0)),
-        },
-        OwnedValue::Blob(b) => match jsonb::from_slice(b) {
-            Ok(_) => Ok(OwnedValue::Integer(1)),
-            Err(_) => Ok(OwnedValue::Integer(0)),
-        },
-        OwnedValue::Null => Ok(OwnedValue::Null),
-        _ => Ok(OwnedValue::Integer(1)),
+
+    #[test]
+    fn test_find_target_array() {
+        let mut val = Val::Array(vec![
+            Val::String("first".to_string()),
+            Val::String("second".to_string()),
+        ]);
+        let path = JsonPath {
+            elements: vec![PathElement::ArrayLocator(0)],
+        };
+
+        match find_target(&mut val, &path) {
+            Some(Target::Array(_, idx)) => assert_eq!(idx, 0),
+            _ => panic!("Expected Array target"),
+        }
+    }
+
+    #[test]
+    fn test_find_target_negative_index() {
+        let mut val = Val::Array(vec![
+            Val::String("first".to_string()),
+            Val::String("second".to_string()),
+        ]);
+        let path = JsonPath {
+            elements: vec![PathElement::ArrayLocator(-1)],
+        };
+
+        match find_target(&mut val, &path) {
+            Some(Target::Array(_, idx)) => assert_eq!(idx, 1),
+            _ => panic!("Expected Array target"),
+        }
+    }
+
+    #[test]
+    fn test_find_target_object() {
+        let mut val = Val::Object(vec![("key".to_string(), Val::String("value".to_string()))]);
+        let path = JsonPath {
+            elements: vec![PathElement::Key("key".to_string())],
+        };
+
+        match find_target(&mut val, &path) {
+            Some(Target::Value(_)) => {}
+            _ => panic!("Expected Value target"),
+        }
+    }
+
+    #[test]
+    fn test_find_target_removed() {
+        let mut val = Val::Object(vec![
+            ("key".to_string(), Val::Removed),
+            ("key".to_string(), Val::String("value".to_string())),
+        ]);
+        let path = JsonPath {
+            elements: vec![PathElement::Key("key".to_string())],
+        };
+
+        match find_target(&mut val, &path) {
+            Some(Target::Value(val)) => assert!(matches!(val, Val::String(_))),
+            _ => panic!("Expected second value, not removed"),
+        }
+    }
+
+    #[test]
+    fn test_mutate_json() {
+        let mut val = Val::Array(vec![Val::String("test".to_string())]);
+        let path = JsonPath {
+            elements: vec![PathElement::ArrayLocator(0)],
+        };
+
+        let result = mutate_json_by_path(&mut val, path, |target| match target {
+            Target::Array(arr, idx) => {
+                arr.remove(idx);
+                "removed"
+            }
+            _ => panic!("Expected Array target"),
+        });
+
+        assert_eq!(result, Some("removed"));
+        assert!(matches!(val, Val::Array(arr) if arr.is_empty()));
+    }
+
+    #[test]
+    fn test_mutate_json_none() {
+        let mut val = Val::Array(vec![]);
+        let path = JsonPath {
+            elements: vec![PathElement::ArrayLocator(0)],
+        };
+
+        let result: Option<()> = mutate_json_by_path(&mut val, path, |_| {
+            panic!("Should not be called");
+        });
+
+        assert_eq!(result, None);
     }
 }

core/translate/expr.rs

@@ -948,6 +948,22 @@ pub fn translate_expr(
                             func_ctx,
                         )
                     }
+                    JsonFunc::JsonRemove => {
+                        if let Some(args) = args {
+                            for arg in args.iter() {
+                                // register containing result of each argument expression
+                                let _ =
+                                    translate_and_mark(program, referenced_tables, arg, resolver)?;
+                            }
+                        }
+                        program.emit_insn(Insn::Function {
+                            constant_mask: 0,
+                            start_reg: target_register + 1,
+                            dest: target_register,
+                            func: func_ctx,
+                        });
+                        Ok(target_register)
+                    }
                 },
                 Func::Scalar(srf) => {
                     match srf {

core/vdbe/mod.rs

@@ -44,7 +44,7 @@ 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_patch,
-    json::json_type,
+    json::json_remove, json::json_type,
 };
 use crate::{resolve_ext_path, Connection, Result, TransactionState, DATABASE_VERSION};
 use datetime::{
@@ -1755,6 +1755,11 @@ impl Program {
                                 let patch = &state.registers[*start_reg + 1];
                                 state.registers[*dest] = json_patch(target, patch)?;
                             }
+                            JsonFunc::JsonRemove => {
+                                state.registers[*dest] = json_remove(
+                                    &state.registers[*start_reg..*start_reg + arg_count],
+                                )?;
+                            }
                         },
                         crate::function::Func::Scalar(scalar_func) => match scalar_func {
                             ScalarFunc::Cast => {

testing/json.test

@@ -676,3 +676,31 @@ do_execsql_test json-patch-abomination {
         '{"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}}}}]}}}}}
+
+do_execsql_test json-remove-1 {
+    select json_remove('{"a": 5, "a": [5,4,3,2,1]}','$.a', '$.a[4]', '$.a[5]', '$.a');
+} {{{}}}
+
+do_execsql_test json-remove-2 {
+    SELECT json_remove('{"a": {"b": {"c": 1, "c": 2}, "b": [1,2,3]}}', '$.a.b.c', '$.a.b[1]');
+} {{{"a":{"b":{"c":2},"b":[1,2,3]}}}}
+
+do_execsql_test json-remove-3 {
+    SELECT json_remove('[1,2,3,4,5]', '$[0]', '$[4]', '$[5]');
+} {{[2,3,4,5]}}
+
+do_execsql_test json-remove-4 {
+    SELECT json_remove('{"arr": [1,2,3,4,5]}', '$.arr[#-1]', '$.arr[#-3]', '$.arr[#-1]');
+} {{{"arr":[1,3]}}}
+
+do_execsql_test json-remove-5 {
+    SELECT json_remove('{}', '$.a');
+} {{{}}}
+
+do_execsql_test json-remove-6 {
+    SELECT json_remove('{"a": [[1,2], [3,4]]}', '$.a[0][1]', '$.a[1]');
+} {{{"a":[[1]]}}}
+
+do_execsql_test json-remove-7 {
+    SELECT json_remove('{"a": 1, "b": [1,2], "c": {"d": 3}}', '$.a', '$.b[0]', '$.c.d');
+} {{{"b":[2],"c":{}}}}