bojana-db commented on PR #58301:
URL: https://github.com/apache/spark/pull/58301#issuecomment-5646045104
@WangGuangxin Thank you for the feature. I have a few high level
notes/questions:
1. The PR description mentions `variant_explode` being the current way to
get the array length, however currently users are able to use
`array_size(variant_get(v, <path>, ARRAY<VARIANT>))` (which also provides
nested array length retrievals) and `array_size(CAST(v as ARRAY<VARIANT>))`
(top level only).
These aren't as efficient (they materialize the array via a full sub-tree
copy rather than reading the count from the variant array header), which is a
good justification for the function, but could we update the "Why are the
changes needed?" section to reflect current user experience?
2. Should we support an optional path argument?
- Checked a few other engines:
- MySQL: supports optional path argument (`JSON_LENGTH(json_doc[,
path])`)
- PostgreSQL: `jsonb_array_length(v)` is top level, but PostgreSQL
path syntax supports `$.path.size()`
- Oracle supports `$.p.size()` path syntax
- Snowflake does not support nested path argument, but most of
their variant functions are top level, so it is consistent with them.
- DuckDB: supports optional path argument (`json_array_length(v[,
path])`)
- Adding the argument aligns with your PR motivation - efficiency. Without
it we would still need to use `variant_get(v, path)` inside the length function
which does the full copy of the returned variant, so the fact that the length
would be later only read from the header, wouldn't bring substantive speedup.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
