kosiew commented on code in PR #23884:
URL: https://github.com/apache/datafusion/pull/23884#discussion_r3690242411
##########
datafusion/spark/src/function/url/url_encode.rs:
##########
@@ -105,22 +93,45 @@ fn spark_url_encode(args: &[ArrayRef]) -> Result<ArrayRef>
{
return exec_err!("`url_encode` expects 1 argument");
}
+ // The percent-encoded form of each value is assembled in a single scratch
buffer
+ // reused across rows, rather than allocating a `String` per row.
+ macro_rules! encode_all {
+ ($array:expr, $builder:expr) => {{
+ let array = $array;
+ let mut builder = $builder;
+ let mut encoded = String::new();
+ for value in array.iter() {
+ match value {
+ Some(value) => {
+ encoded.clear();
+ encoded.extend(byte_serialize(value.as_bytes()));
+ builder.append_value(&encoded);
+ }
+ None => builder.append_null(),
+ }
+ }
+ Ok(Arc::new(builder.finish()) as ArrayRef)
+ }};
+ }
+
match &args[0].data_type() {
Review Comment:
Could we add direct Rust unit tests for the new `LargeUtf8` and `Utf8View`
builder paths in `url_encode` and `url_decode`? The SLTs already cover these
input types, but the Rust unit tests mostly exercise the `Utf8` path. A small
test using `LargeStringArray` and `StringViewArray` would help guard these new
optimized branches more directly.
--
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]
