-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add Spark json_array_length function #11946
base: main
Are you sure you want to change the base?
feat: Add Spark json_array_length function #11946
Conversation
a971121
to
a5c0d3b
Compare
✅ Deploy Preview for meta-velox canceled.
|
✅ Deploy Preview for meta-velox canceled.
|
struct JsonArrayLengthFunction { | ||
VELOX_DEFINE_FUNCTION_TYPES(T); | ||
|
||
FOLLY_ALWAYS_INLINE bool call(int32_t& len, const arg_type<Json>& json) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
Change const arg_type<Json>&
to const arg_type<Varchar>&
and remove the above corresponding header.
I think JsonType is just for presto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Added some comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating. Added some comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Added several nits on the doc.
75dc918
to
dbd4e2f
Compare
The 'json_array_length' functions of Presto and Spark are similar, however
Presto returns int64_t, while Spark returns int32_t. In this PR, the
'JsonArrayLengthFunction' implementation is extracted to 'velox/functions/lib'
and registered with differenct return types for Presto and Spark.
Spark doc: https://spark.apache.org/docs/latest/api/sql/#json_array_length