cloudwego / cloudwego/sonic-rs

Inconsistent behavior in `get_many` for non-existent paths

Open
#201 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
920
Forks
68
Avg merge
2h 5m
Merged PRs (30d)
1

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**

```rust
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:

```rust
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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.