jayzhan211 commented on code in PR #25874:
URL: https://github.com/apache/datafusion/pull/25874#discussion_r4365516231
##########
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:
This fast path runs `substr` over every dictionary value, so its cost
follows `values().len()` rather than the row count. Parquet reuses one
dictionary for every batch of a column chunk, and `filter` keeps the full
values. I should have caught this in an earlier round, sorry. On 8192 rows with
scalar start/length it's 3.4x slower than decoding at 50k values and 13.7x at
200k. Falling back to the row-wise path when the dictionary is larger than the
batch brings that to 1.05x and keeps the wins at 16/1k/8k values. I couldn't
make it show up end-to-end on TPC-H or ClickBench, so a follow-up is fine if
you'd rather land this as is:
```diff
if let ColumnarValue::Array(array) = &args.args[0]
&& matches!(array.data_type(), DataType::Dictionary(_, _))
+ && array.as_any_dictionary().values().len() <= array.len()
&& args.args[1..]
```
--
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]
