mattp5657 opened a new issue, #4308:
URL: https://github.com/apache/iggy/issues/4308
### Description
`core/connectors/sdk/src/convert.rs::owned_value_to_serde_json` takes
`&simd_json::OwnedValue` and recursively allocates a fresh `String`
for every string leaf (`s.to_string()`) and every object key
(`k.to_string()`) it visits:
```rust
pub fn owned_value_to_serde_json(value: &simd_json::OwnedValue) ->
serde_json::Value {
match value {
simd_json::OwnedValue::Static(s) => match s { /* ... */ },
simd_json::OwnedValue::String(s) =>
serde_json::Value::String(s.to_string()),
simd_json::OwnedValue::Array(arr) => {
serde_json::Value::Array(arr.iter().map(owned_value_to_serde_json).collect())
}
simd_json::OwnedValue::Object(obj) => {
let map: serde_json::Map<String, serde_json::Value> = obj
.iter()
.map(|(k, v)| (k.to_string(), owned_value_to_serde_json(v)))
.collect();
serde_json::Value::Object(map)
}
}
}
```
At most call sites the caller already owns the `OwnedValue` outright
and drops it immediately after converting. Converting between
`OwnedValue` and `serde_json::Value` means rebuilding the tree either
way, since they're unrelated types, but the leaf strings and object
keys don't need to ride along with that rebuild: they're already-owned
heap allocations one line away from being dropped, so `.to_string()`
copies their bytes into a new allocation just to throw the original
away right after.
### Affected area / component
Connectors
### Proposed solution
Add a consuming variant, additive (no breaking signature change):
```rust
pub fn owned_value_into_serde_json(value: simd_json::OwnedValue) ->
serde_json::Value {
match value {
simd_json::OwnedValue::Static(s) => match s { /* unchanged, Copy
types */ },
simd_json::OwnedValue::String(s) =>
serde_json::Value::String(s.into()),
simd_json::OwnedValue::Array(arr) => {
serde_json::Value::Array(arr.into_iter().map(owned_value_into_serde_json).collect())
}
simd_json::OwnedValue::Object(obj) => {
let map: serde_json::Map<String, serde_json::Value> = obj
.into_iter()
.map(|(k, v)| (k.into(), owned_value_into_serde_json(v)))
.collect();
serde_json::Value::Object(map)
}
}
}
```
Keep the existing borrow-based function for call sites that only have
a `&OwnedValue`. Swap `opensearch_sink`, `http_sink`, both
`surrealdb_sink` sites, and `avro.rs` to the consuming function
directly. Leave `s3_sink` and `delta_sink` for a follow-up, since they
need their callers restructured to own the payload first (see "Where
it occurs"). `convert.rs` already has an 11-case test module covering
every `OwnedValue` variant for the existing borrow-based function; the
new function's tests can mirror those directly rather than being
written from scratch. Should land as its own PR, not folded into an
unrelated sink's branch.
### Alternatives considered
_No response_
### Contribution
- [x] I'm willing to submit a pull request to implement this feature
### Good first issue
- [ ] I think this could be a good first issue for a new contributor
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
