-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(datafusion-spark): implement spark compatible unhex function
#19909
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| Self { | ||
| signature: Signature::coercible(vec![string], Volatility::Immutable), | ||
| aliases: vec![], |
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.
Remove aliases if there are none
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
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.
Can we move all these tests to SLTs?
| } | ||
| } | ||
|
|
||
| pub fn spark_unhex(args: &[ColumnarValue]) -> Result<ColumnarValue, DataFusionError> { |
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.
This doesn't need to be pub
| if args.len() != 1 { | ||
| return exec_err!("unhex tasks exactly 1 argument, but got: {}", args.len()); | ||
| } |
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.
| if args.len() != 1 { | |
| return exec_err!("unhex tasks exactly 1 argument, but got: {}", args.len()); | |
| } | |
| let arg = take_function_args("unhex", &args.args)?; |
| I: Iterator<Item = Option<T>>, | ||
| T: AsRef<str>, | ||
| { | ||
| let mut builder = BinaryBuilder::with_capacity(len, len * 32); |
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.
Why 32?
| } | ||
| } | ||
|
|
||
| fn unhex_common(bytes: &[u8], out: &mut Vec<u8>) -> bool { |
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.
Might be good to leave a simple comment on the function explaining the outputs (writes into out, returns boolean indicating if valid or not)
Which issue does this PR close?
Part of: #15914
Rationale for this change
Implement spark compatible unhex functions:
https://spark.apache.org/docs/latest/api/sql/index.html#unhex
What changes are included in this PR?
Are these changes tested?
Yes. UTs and SLT added.
Are there any user-facing changes?
No.