Improve error handling and add some more types for proper downcasting #4352
Conversation
github-actions
bot
added
core
| @@ -720,7 +722,7 @@ fn get_dict_value<K: ArrowDictionaryKeyType>( | |||
| array: &ArrayRef, | |||
| index: usize, | |||
| ) -> (&ArrayRef, Option<usize>) { | |||
| let dict_array = as_dictionary_array::<K>(array); | |||
| let dict_array = as_dictionary_array::<K>(array).unwrap(); | |||
There was a problem hiding this comment.
for other reviewers, the rationale here is that arrow cast kernels used previously panic internally if the type is incorrect -- the ones in DataFusion return an error instead
| .as_any() | ||
| .downcast_ref::<PrimitiveArray<Int32Type>>() | ||
| .unwrap(), | ||
| as_primitive_array::<Int32Type>(batch.column(*i)).unwrap(), |
| .ok_or(DataFusionError::Internal( | ||
| "median! macro failed to cast array to expected type".to_string(), | ||
| ))?; | ||
| let array = as_primitive_array::<$TY>(&sorted)?; |
| @@ -50,43 +53,6 @@ macro_rules! downcast_string_arg { | |||
| }}; | |||
| } | |||
|
|
|||
| macro_rules! downcast_primitive_array_arg { | |||
There was a problem hiding this comment.
I reviewed the changes in this module carefully and they look good to me -- thank you @retikulum
|
Benchmark runs are scheduled for baseline = 0496904 and contender = da54fa5. da54fa5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
This is discussed in #3152.
Rationale for this change
This is the new PR of improving error handling for downcasting. In #4261 we found out that there is a
downcast_value!macro which also covers our casting functions. Implementingas_xxx_arrayfunction withdowncast_value!is nearly decided for now for future PRs.What changes are included in this PR?