Rich-T-kid commented on code in PR #23187:
URL: https://github.com/apache/datafusion/pull/23187#discussion_r3638691922
##########
datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs:
##########
@@ -1392,6 +1425,107 @@ mod tests {
}
}
+ // Regression for https://github.com/apache/datafusion/issues/23127:
+ // In a multi-column group-by there can be more than 128 groups while an
+ // Int8 dictionary column still has only a few distinct values. The toll
+ // row path must emit 129 groups without overflowing the Int8 key range.
+ // This also documents that the inner GroupColumn does NOT deduplicate dict
+ // values — each group gets its own slot, so values().len() == n_groups.
+ #[test]
+ fn multi_col_groupby_dict_many_groups_two_values() {
+ use arrow::array::{AsArray, DictionaryArray, Int16Array};
+ use arrow::datatypes::Int16Type;
+
+ let n_groups = 129_usize;
+ let dict_vocab: ArrayRef = Arc::new(StringArray::from(vec!["cat",
"dog"]));
+
+ // Each row has a unique label (forcing a new group) and alternates
+ // between the two dictionary values. Int16 keys are used so that
+ // 129 groups don't hit the Int8 overflow limit (i8::MAX = 127).
+ let labels: ArrayRef = Arc::new(StringArray::from(
+ (0..n_groups).map(|i| format!("g{i}")).collect::<Vec<_>>(),
+ ));
+ let dict_keys = Int16Array::from(
+ (0..n_groups)
+ .map(|i| Some((i % 2) as i16))
+ .collect::<Vec<_>>(),
+ );
+ let categories: ArrayRef = Arc::new(DictionaryArray::<Int16Type>::new(
Review Comment:
Good catch! I forgot to revert this.
--
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]
