Jefffrey commented on code in PR #23930:
URL: https://github.com/apache/datafusion/pull/23930#discussion_r3665360364
##########
datafusion/functions/src/unicode/initcap.rs:
##########
@@ -83,63 +84,84 @@ impl ScalarUDFImpl for InitcapFunc {
}
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- if let DataType::Utf8View = arg_types[0] {
- Ok(DataType::Utf8View)
- } else {
- utf8_to_str_type(&arg_types[0], "initcap")
- }
+ transform_leaf_type_preserving_encoding(&arg_types[0], &|data_type| {
+ if let DataType::Utf8View = data_type {
+ Ok(DataType::Utf8View)
+ } else {
+ utf8_to_str_type(data_type, "initcap")
+ }
Review Comment:
```suggestion
data_type
```
i think we can clean this up like so, since we're just preserving input utf8
type
##########
datafusion/functions/src/unicode/character_length.rs:
##########
@@ -59,11 +62,16 @@ impl Default for CharacterLengthFunc {
impl CharacterLengthFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::uniform(
- 1,
- vec![Utf8, LargeUtf8, Utf8View],
Review Comment:
i think this allowed any type;
```sql
> select character_length(10);
+-----------------------------+
| character_length(Int64(10)) |
+-----------------------------+
| 2 |
+-----------------------------+
1 row(s) fetched.
Elapsed 0.004 seconds.
```
##########
datafusion/functions/src/unicode/reverse.rs:
##########
@@ -56,11 +59,16 @@ impl Default for ReverseFunc {
impl ReverseFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::uniform(
- 1,
- vec![Utf8View, Utf8, LargeUtf8],
Review Comment:
similarly here it used to accept anything
##########
datafusion/functions/src/unicode/initcap.rs:
##########
@@ -83,63 +84,84 @@ impl ScalarUDFImpl for InitcapFunc {
}
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- if let DataType::Utf8View = arg_types[0] {
- Ok(DataType::Utf8View)
- } else {
- utf8_to_str_type(&arg_types[0], "initcap")
- }
+ transform_leaf_type_preserving_encoding(&arg_types[0], &|data_type| {
+ if let DataType::Utf8View = data_type {
+ Ok(DataType::Utf8View)
+ } else {
+ utf8_to_str_type(data_type, "initcap")
+ }
+ })
}
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
let arg = &args.args[0];
// Scalar fast path - handle directly without array conversion
if let ColumnarValue::Scalar(scalar) = arg {
- return match scalar {
- ScalarValue::Utf8(None)
- | ScalarValue::LargeUtf8(None)
- | ScalarValue::Utf8View(None) => Ok(arg.clone()),
- ScalarValue::Utf8(Some(s)) => {
- let mut result = String::new();
- initcap_string(s, &mut result);
- Ok(ColumnarValue::Scalar(ScalarValue::Utf8(Some(result))))
- }
- ScalarValue::LargeUtf8(Some(s)) => {
- let mut result = String::new();
- initcap_string(s, &mut result);
-
Ok(ColumnarValue::Scalar(ScalarValue::LargeUtf8(Some(result))))
- }
- ScalarValue::Utf8View(Some(s)) => {
- let mut result = String::new();
- initcap_string(s, &mut result);
-
Ok(ColumnarValue::Scalar(ScalarValue::Utf8View(Some(result))))
- }
- other => {
- exec_err!(
- "Unsupported data type {:?} for function `initcap`",
- other.data_type()
- )
- }
- };
+ return Ok(ColumnarValue::Scalar(initcap_scalar(scalar)?));
}
// Array path
- let args = &args.args;
- match args[0].data_type() {
- DataType::Utf8 => make_scalar_function(initcap::<i32>,
vec![])(args),
- DataType::LargeUtf8 => make_scalar_function(initcap::<i64>,
vec![])(args),
- DataType::Utf8View => make_scalar_function(initcap_utf8view,
vec![])(args),
- other => {
- exec_err!("Unsupported data type {other:?} for function
`initcap`")
- }
- }
+ let ColumnarValue::Array(array) = arg else {
Review Comment:
would prefer to refactor this function into a match statement so we dont
need this destructuring (which introduces an unreachable)
--
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]
