feat: implement json_get_array UDF #36
Conversation
|
@samuelcolvin I tried to get it to work using the JsonUnion struct contained in this lib but ended up making my own struct as it was easier to work with initially. I will try to make it work with the JsonUnion struct, any hints would be appreciated |
|
nvm figured it out but it's a tiny bit hacky to support both the jiter index based access syntax and the new udf. It might be better to split this up... |
src/common_union.rs
Outdated
| @@ -111,6 +114,25 @@ impl FromIterator<Option<JsonUnionField>> for JsonUnion { | |||
| } | |||
| } | |||
|
|
|||
| /// Tries to collect an array represented as a string to a `JsonUnion` | |||
| impl FromIterator<Option<String>> for JsonUnion { | |||
There was a problem hiding this comment.
I don't really understand this, if we're just collecting some optional strings, how can we know they all represent arrays?
There was a problem hiding this comment.
I don't think this is the right way to go about what (I think) you're trying to do.
If you need this logic, better to put it where you need it.
src/json_get_array.rs
Outdated
| } | ||
| } | ||
|
|
||
| fn jiter_json_get_array(json_data: Option<&str>, path: &[JsonPath]) -> Result<String, GetError> { |
There was a problem hiding this comment.
I think you want to build up and return a UnionArray here, not return a string.
|
Sorry for the noise I tried a couple of different appoaches but I think the final one I landed on is ok. I implemented it using the JsonUnion struct by adding a more refined type for the If this approach is acceptable, I would also proceed adding docs. Anything else I should include in this PR? |
|
As I understand according to apache/arrow-rs#6218 it is not possible to extract arrays from arrow union types right? Otherwise it would be possible to support something like the statement below which would completely remove the need for the SELECT json_get('{"foo": [1, 2, 3]}', foo)::text[]; |
| @@ -93,24 +96,6 @@ impl JsonUnion { | |||
| } | |||
| } | |||
|
|
|||
| /// So we can do `collect::<JsonUnion>()` | |||
| impl FromIterator<Option<JsonUnionField>> for JsonUnion { | |||
There was a problem hiding this comment.
moved down to the JsonUnionField definition
Implements json_get_array which unwraps the JSON array into an arrow array containing the raw JSON strings.
The current behavior will leave the contents of the underlying JSON data unparsed so it needs additional parsing when accessing the individual array elements