alamb commented on code in PR #4352:
URL: https://github.com/apache/arrow-datafusion/pull/4352#discussion_r1032783834
##########
datafusion/common/src/scalar.rs:
##########
@@ -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();
Review 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
##########
datafusion/physical-expr/src/string_expressions.rs:
##########
@@ -50,43 +53,6 @@ macro_rules! downcast_string_arg {
}};
}
-macro_rules! downcast_primitive_array_arg {
Review Comment:
I reviewed the changes in this module carefully and they look good to me --
thank you @retikulum
##########
datafusion/physical-expr/src/aggregate/median.rs:
##########
@@ -102,12 +103,7 @@ macro_rules! median {
return Ok(ScalarValue::Null);
}
let sorted = sort(&combined, None)?;
- let array = sorted
- .as_any()
- .downcast_ref::<PrimitiveArray<$TY>>()
- .ok_or(DataFusionError::Internal(
- "median! macro failed to cast array to expected
type".to_string(),
- ))?;
+ let array = as_primitive_array::<$TY>(&sorted)?;
Review Comment:
much nicer
##########
datafusion/core/tests/custom_sources.rs:
##########
@@ -162,18 +163,10 @@ impl ExecutionPlan for CustomExecutionPlan {
.map(|i| ColumnStatistics {
null_count: Some(batch.column(*i).null_count()),
min_value: Some(ScalarValue::Int32(aggregate::min(
- batch
- .column(*i)
- .as_any()
- .downcast_ref::<PrimitiveArray<Int32Type>>()
- .unwrap(),
+
as_primitive_array::<Int32Type>(batch.column(*i)).unwrap(),
Review Comment:
👍
--
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]
