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


##########
datafusion/functions/src/string/lower.rs:
##########
@@ -197,4 +195,31 @@ mod tests {
 
         to_lower(input, expected)
     }
+
+    #[test]
+    fn lower_utf8view() -> Result<()> {
+        let input = Arc::new(StringViewArray::from(vec![
+            Some("ARROW"),
+            None,
+            Some("TSCHÜSS"),
+        ])) as ArrayRef;
+
+        let expected = Arc::new(StringViewArray::from(vec![
+            Some("arrow"),
+            None,
+            Some("tschüss"),
+        ])) as ArrayRef;
+
+        to_lower(input, expected)
+    }
+
+    #[test]
+    fn lower_return_type_dictionary_utf8view() -> Result<()> {

Review Comment:
   For symmetry, should we add an analogous test case for `upper`?



##########
datafusion/functions/src/string/common.rs:
##########
@@ -331,6 +332,22 @@ fn string_trim<T: OffsetSizeTrait, Tr: Trimmer>(args: 
&[ArrayRef]) -> Result<Arr
     }
 }
 
+pub(crate) fn case_conversion_return_type(
+    arg_type: &DataType,
+    name: &str,
+) -> Result<DataType> {
+    match arg_type {
+        DataType::Utf8View | DataType::BinaryView => Ok(DataType::Utf8View),

Review Comment:
   I believe `BinaryView` will be rejected by the type signatures for these 
functions, so probably clearer if we don't handle it.



##########
datafusion/sqllogictest/test_files/functions.slt:
##########
@@ -445,6 +445,21 @@ SELECT upper(arrow_cast('árvore ação αβγ', 
'Dictionary(Int32, Utf8)'))
 ----
 ÁRVORE AÇÃO ΑΒΓ
 
+query T
+SELECT arrow_typeof(upper('foo'))
+----
+Utf8
+
+query T
+SELECT arrow_typeof(upper(arrow_cast('foo', 'LargeUtf8')))
+----
+LargeUtf8
+
+query T
+SELECT arrow_typeof(upper(arrow_cast('foo', 'Utf8View')))

Review Comment:
   Add SLT tests for dictionary with a Utf8View value type?



##########
datafusion/functions/src/string/common.rs:
##########
@@ -331,6 +332,22 @@ fn string_trim<T: OffsetSizeTrait, Tr: Trimmer>(args: 
&[ArrayRef]) -> Result<Arr
     }
 }
 
+pub(crate) fn case_conversion_return_type(

Review Comment:
   This doesn't seem specific to case conversion; we probably need a more 
generic facility for finding the return return type for a function that takes a 
string. But we can handle that later.



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