nathanb9 commented on code in PR #22315:
URL: https://github.com/apache/datafusion/pull/22315#discussion_r3255727517
##########
datafusion/functions/src/datetime/date_bin.rs:
##########
@@ -691,20 +723,24 @@ fn date_bin_impl(
T: ArrowTimestampType,
{
let array = as_primitive_array::<T>(array)?;
- let scale = match T::UNIT {
- Nanosecond => 1,
- Microsecond => NANOS_PER_MICRO,
- Millisecond => NANOS_PER_MILLI,
- Second => NANOSECONDS,
- };
-
- let result: PrimitiveArray<T> = array.try_unary(|val| {
- stride_fn(stride, val * scale, origin)
- .map(|binned| binned / scale)
- .map_err(|e| {
-
arrow::error::ArrowError::ComputeError(e.to_string())
- })
- })?;
+ let scale = timestamp_scale::<T>();
+
+ let result: PrimitiveArray<T> = array
+ .try_unary(|val| {
+ let scaled =
+ scale_timestamp_to_nanos(val, scale).map_err(|e| {
+
arrow::error::ArrowError::ComputeError(e.to_string())
+ })?;
+
+ stride_fn(stride, scaled, origin)
+ .map(|binned| binned / scale)
+ .map_err(|e| {
Review Comment:
`ArrowError::ComputeError("Execution error: DATE_BIN source timestamp...")`
gets wrapped a second time into `DataFusionError::Execution(...)` which i htink
will produce the triple nested message.
##########
datafusion/functions/src/datetime/date_bin.rs:
##########
@@ -1066,6 +1102,24 @@ mod tests {
res.err().unwrap().strip_backtrace(),
"This feature is not implemented: DATE_BIN only supports literal
values for the origin argument, not arrays"
);
+
Review Comment:
Could you add simple test for the array path?
just like:
```rust
let input = TimestampSecondArray::from(vec![Some(i64::MAX)]);
let args = vec![
ColumnarValue::Scalar(ScalarValue::IntervalMonthDayNano(...)),
ColumnarValue::Array(Arc::new(input)),
ColumnarValue::Scalar(ScalarValue::TimestampNanosecond(Some(0), None)),
];
let res = invoke_date_bin_with_args(args, 1, return_field);
assert!(res.err().unwrap().strip_backtrace().contains(
"DATE_BIN source timestamp 9223372036854775807 cannot be represented
in nanoseconds"
));
```
related to first comment because this might help clear up what you expect
the result to look like
--
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]
