neilconway commented on code in PR #26377:
URL: https://github.com/apache/datafusion/pull/26377#discussion_r4285614881


##########
datafusion/functions/src/string/common.rs:
##########
@@ -390,38 +390,48 @@ fn unicode_case(s: &str, lower: bool) -> String {
     }
 }
 
-/// Like [`unicode_case`], but uppercases into `buf` so its allocation is 
reused
-/// across rows. Lowercasing still allocates, see [`write_unicode_case`].
+/// Appends `s` to `builder` with its case converted.
+///
+/// An ASCII value is converted byte by byte straight into the builder. Other
+/// values go through `str::to_lowercase` / `str::to_uppercase`, which allocate
+/// a `String` but convert any leading run of ASCII bytes in bulk.
 #[inline]
-fn unicode_case_into<'a>(s: &str, lower: bool, buf: &'a mut String) -> &'a str 
{
-    if lower {
-        *buf = s.to_lowercase();
-    } else {
-        buf.clear();
-        buf.extend(s.chars().flat_map(char::to_uppercase));
+fn append_case_converted<O: OffsetSizeTrait>(

Review Comment:
   I actually started by adding `try_append_byte_map` to 
`BulkNullStringArrayBuilder ` originally :) But I felt like the PR was large 
enough as it was. I agree this makes sense but I'd prefer to defer to a 
followup PR.



-- 
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]

Reply via email to