Jefffrey commented on code in PR #17131:
URL: https://github.com/apache/datafusion/pull/17131#discussion_r2271916893


##########
datafusion/functions/src/string/ascii.rs:
##########
@@ -30,7 +30,7 @@ use std::sync::Arc;
 
 #[user_doc(
     doc_section(label = "String Functions"),
-    description = "Returns the Unicode character code of the first character 
in a string.",
+    description = "Returns the first Unicode scalar value of a string.",

Review Comment:
   I'm a bit curious about this wording change as the original wording seems 
more intuitive, at least to me 🤔 



##########
datafusion/sqllogictest/test_files/expr.slt:
##########


Review Comment:
   GitHub is showing this diff as binary to me; what actual changes were made 
here?



##########
datafusion/functions/src/string/chr.rs:
##########
@@ -47,22 +47,14 @@ pub fn chr(args: &[ArrayRef]) -> Result<ArrayRef> {
     for integer in integer_array {
         match integer {
             Some(integer) => {
-                if integer == 0 {
-                    return exec_err!("null character not permitted.");
-                } else if integer < 0 {
-                    return exec_err!("negative input not permitted.");
-                } else {
-                    match core::char::from_u32(integer as u32) {
-                        Some(c) => {
-                            builder.append_value(c.encode_utf8(&mut buf));
-                        }
-                        None => {
-                            return exec_err!(
-                                "requested character too large for encoding."
-                            );
-                        }
+                if let Ok(u) = u32::try_from(integer) {
+                    if let Some(c) = core::char::from_u32(u) {
+                        builder.append_value(c.encode_utf8(&mut buf));
+                        continue;
                     }
                 }
+
+                return exec_err!("invalid Unicode scalar value: {integer}");

Review Comment:
   This is a nice change with better error reporting 👍 



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to