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