feat(spark): Implement Spark functions url_encode and url_decode
#17399
Conversation
| /// | ||
| fn decode(value: &str) -> Result<String> { | ||
| // Check if the string has valid percent encoding | ||
| // TODO: Support `try_url_decode` |
There was a problem hiding this comment.
Is this TODO still valid? Should we open an issue and link to it if so?
| } | ||
|
|
||
| /// Replace b'+' with b' ' | ||
| fn replace_plus(input: &[u8]) -> Cow<'_, [u8]> { |
There was a problem hiding this comment.
Could we add a test case that covers this replacement?
| ); | ||
| } | ||
| match arg_types[0] { | ||
| DataType::Utf8 | DataType::Utf8View => Ok(DataType::Utf8), |
There was a problem hiding this comment.
It looks like you're collecting to a StringView for input type of Utf8View, so shouldn't this be more like
DataType::Utf8 | DataType::Utf8View | DataType::LargeUtf8 => Ok(arg_types[0].clone())
| ); | ||
| } | ||
| match arg_types[0] { | ||
| DataType::Utf8 | DataType::Utf8View => Ok(DataType::Utf8), |
There was a problem hiding this comment.
Same question as for decode
| /// * `Ok(String)` - The encoded string | ||
| /// | ||
| fn encode(value: &str) -> Result<String> { | ||
| Ok(byte_serialize(value.as_bytes()).collect::<String>()) |
There was a problem hiding this comment.
So for encoding you don't need to do the inverse operations you do in decoding? Like the + -> ' ' manipulation? I am unfamiliar with the specific needs here.
| # For more information, please see: | ||
| # https://github.com/apache/datafusion/issues/15914 | ||
| query T | ||
| SELECT url_decode('https%3A%2F%2Fspark.apache.org'::string); |
There was a problem hiding this comment.
I am unfamiliar with the slt files, but if there was a way to have these do all 3 types of input (utf8, utf8view, largeutf8) that would be nice.
Which issue does this PR close?
datafusion-sparkSpark Compatible Functions #15914Rationale for this change
What changes are included in this PR?
Implement Spark functions
url_encodeandurl_decodeAre these changes tested?
Yes
Are there any user-facing changes?
Yes