alamb commented on code in PR #6654:
URL: https://github.com/apache/arrow-datafusion/pull/6654#discussion_r1227334439
##########
datafusion/physical-expr/src/datetime_expressions.rs:
##########
@@ -294,16 +294,24 @@ pub fn date_trunc(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
}
"second" => {
// trunc to second
- let mill = ScalarValue::TimestampNanosecond(
Review Comment:
I think we would have to change the values returned by
https://github.com/apache/arrow-datafusion/pull/6654/files#diff-67ae8808785b2e651767d7ff67dd7c53be04ca098857b52c82ed19927e071cdaL322
as well as the type returned by `BuiltInScalarFunction::return_type`
otherwise we'll be in the situation again where the scalar implementation
does something different than the array implementation
However, I was surprised that there were no tests that failed which reveals
a gap in our test coverage. I wrote the test in
https://github.com/apache/arrow-datafusion/pull/6655 which does pass on `main`
but fails on this PR
--
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]
