[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2767: minor: panic rather than fail silently on bad dictionary in hash join

[GitBox <gi...@apache.org>]

alamb commented on code in PR #2767:
URL: https://github.com/apache/arrow-datafusion/pull/2767#discussion_r903761910


##########
datafusion/core/src/physical_plan/hash_join.rs:
##########
@@ -961,42 +961,35 @@ macro_rules! equal_rows_elem_with_string_dict {
         let (left_values, left_values_index) = {
             let keys_col = left_array.keys();
             if keys_col.is_valid($left) {
-                let values_index = keys_col.value($left).to_usize().ok_or_else(|| {
-                    DataFusionError::Internal(format!(
-                    "Can not convert index to usize in dictionary of type creating group by value {:?}",
-                    keys_col.data_type()
-                ))
-                });
-
-                match values_index {
-                    Ok(index) => (as_string_array(left_array.values()), Some(index)),
-                    _ => (as_string_array(left_array.values()), None)

Review Comment:
   this ignores invalid data in DictionaryArrays (treats them as though they were NULLs) -- I think `panic` is better



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org