You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/06/12 23:22:33 UTC

[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6654: fix: 'datatrunc' return inconsistent type

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

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