alamb commented on a change in pull request #9625:
URL: https://github.com/apache/arrow/pull/9625#discussion_r587742457



##########
File path: rust/datafusion/tests/sql.rs
##########
@@ -2051,13 +2054,19 @@ async fn test_string_expressions() -> Result<()> {
     test_expression!("character_length('chars')", "5");
     test_expression!("character_length('josé')", "4");
     test_expression!("character_length(NULL)", "NULL");
+    test_expression!("chr(CAST(120 AS int))", "x");
+    test_expression!("chr(CAST(128175 AS int))", "💯");

Review comment:
       💯  indeed!

##########
File path: rust/datafusion/src/physical_plan/functions.rs
##########
@@ -1033,6 +1163,54 @@ mod tests {
 
     #[test]
     fn test_functions() -> Result<()> {
+        test_function!(
+            Ascii,
+            &[lit(ScalarValue::Utf8(Some("x".to_string())))],
+            Ok(Some(120)),
+            i32,
+            Int32,
+            Int32Array
+        );
+        test_function!(
+            Ascii,
+            &[lit(ScalarValue::Utf8(Some("ésoj".to_string())))],
+            Ok(Some(233)),
+            i32,
+            Int32,
+            Int32Array
+        );
+        test_function!(
+            Ascii,
+            &[lit(ScalarValue::Utf8(Some("💯".to_string())))],
+            Ok(Some(128175)),

Review comment:
       It is strange to me that a function called `ascii` can return something 
larger than `127`. However, it seems that quirkiness / awesomeness came from 
`postgres` :)
   
   ```
   alamb=# select ascii('💯');
    ascii  
   --------
    128175
   (1 row)
   
   ```
   👍 

##########
File path: rust/datafusion/src/physical_plan/string_expressions.rs
##########
@@ -658,32 +709,9 @@ pub fn rpad<T: StringOffsetSizeTrait>(args: &[ArrayRef]) 
-> Result<ArrayRef> {
             Ok(Arc::new(result) as ArrayRef)
         }
         3 => {
-            let string_array: &GenericStringArray<T> = args[0]
-                .as_any()
-                .downcast_ref::<GenericStringArray<T>>()
-                .ok_or_else(|| {
-                    DataFusionError::Internal(
-                        "could not cast string to StringArray".to_string(),
-                    )
-                })?;
-
-            let length_array: &Int64Array = args[1]
-                .as_any()
-                .downcast_ref::<Int64Array>()
-                .ok_or_else(|| {
-                    DataFusionError::Internal(
-                        "could not cast length to Int64Array".to_string(),
-                    )
-                })?;
-
-            let fill_array: &GenericStringArray<T> = args[2]
-                .as_any()
-                .downcast_ref::<GenericStringArray<T>>()
-                .ok_or_else(|| {
-                    DataFusionError::Internal(
-                        "could not cast fill to StringArray".to_string(),
-                    )
-                })?;
+            let string_array = downcast_string_arg!(args[0], "string", T);

Review comment:
       these macros certainly make the code nicer to read. 👍 




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

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


Reply via email to