-
Notifications
You must be signed in to change notification settings - Fork 59
Description
Hello, first of all I want to say that I'm getting a staggering performance using the get_many_unchecked in a field extraction pipeline. However, I've encountered some quirky behaviors, that I feel are likely bugs. get_by_schema doesn't come with similar issues, but based on my benchmarks it is much slower.
Describe the bug
get_many/get_many_unchecked has inconsistent behavior when querying paths that don't exist. Querying a missing key in an empty object returns Err, but querying a missing key in a non-empty object returns Ok([None]). This causes the entire multi-path query to fail when any path encounters an empty object/array.
To Reproduce
use sonic_rs::{get_many, pointer, PointerTree};
fn main() {
// Case 1: Non-empty object with missing key -> Ok([None])
let json = r#"{"a": {"d": 1}, "b": {"c": "foo"}}"#;
let mut tree = PointerTree::new();
tree.add_path(&pointer!["a", "c"]); // "a" exists but has no key "c"
let result = get_many(json, &tree);
assert!(result.is_ok());
assert!(result.unwrap()[0].is_none()); // Works as documented
// Case 2: Empty object with missing key -> Err
let json = r#"{"a": {}, "b": {"c": "foo"}}"#;
let mut tree = PointerTree::new();
tree.add_path(&pointer!["a", "c"]); // "a" is empty
let result = get_many(json, &tree);
assert!(result.is_err()); // Unexpected. My intuition: should be Ok([None])
}Both cases query a path that doesn't exist (a.c), but they behave differently based on whether the intermediate object is empty or not.
Expected behavior
According to the documentation: "If a key is unknown, the value at the corresponding position in Vec will be None."
Both cases should return Ok([None]) since the path a.c doesn't exist in either case.
Following this logic, I feel that out-of-bounds array accesses shouldn't fail the entire get_many call. Just return None for a missing item. This would prevent a single missing path from failing an entire multi-path query:
let json = r#"{"a": [1, 2], "b": {"c": "found"}, "d": {}}"#;
let mut tree = PointerTree::new();
tree.add_path(&pointer!["a", 99]); // out of bounds
tree.add_path(&pointer!["b", "c"]); // valid path
tree.add_path(&pointer!["b", "d"]); // invalid path
tree.add_path(&pointer!["d", "e"]); // invalid path, non-existent object
let result = get_many(json, &tree);
// Currently: Err (entire query fails)
// Expected: Ok([None, Some("found"), None, None])sonic-rs version:
Both 0.5.6 and master exhibit same behavior