Rich-T-kid commented on code in PR #25874:
URL: https://github.com/apache/datafusion/pull/25874#discussion_r4368120521


##########
datafusion/functions/src/unicode/substr.rs:
##########
@@ -112,10 +123,42 @@ impl ScalarUDFImpl for SubstrFunc {
     }
 
     fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
-        Ok(arg_types[0].clone())
+        Ok(match &arg_types[0] {
+            DataType::Dictionary(key_type, value_type) if 
!is_wide_key(key_type) => {
+                value_type.as_ref().clone()
+            }
+            other => other.clone(),
+        })
     }
 
     fn invoke_with_args(&self, args: ScalarFunctionArgs) -> 
Result<ColumnarValue> {
+        if let DataType::Dictionary(key_type, value_type) = 
args.args[0].data_type()
+            && !is_wide_key(&key_type)
+        {
+            // Per-row start/length can produce more distinct values than a
+            // narrow key type can index, so decode instead.
+            let mut args = args.args;
+            args[0] = args[0].cast_to(&value_type, None)?;
+            return make_scalar_function(substr, vec![])(&args);
+        }
+
+        if let ColumnarValue::Array(array) = &args.args[0]
+            && matches!(array.data_type(), DataType::Dictionary(_, _))

Review Comment:
   @jayzhan211 addressed in c5ca332c7. The dictionary-values fast path now only 
runs when the dictionary is no larger than the batch. Ready for another look.



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