jhorstmann commented on pull request #9040:
URL: https://github.com/apache/arrow/pull/9040#issuecomment-752424442
Cool! A few small comments:
- In postgres the argument order is the other way around
[`date_trunc('week', timestamp)`][1]. I haven't compared with other databases,
but following postgres makes sense to me most of the time :)
- A few unit tests, especially for the week truncation around the
beginning/end of a year would be nice. Maybe the test input could be prepared
using `to_timestamp` so the test is easier to review.
- In most usages, the date part parameter would be a literal and it might
be worthwhile to optimize for that. Doesn't fit that nicely into the current
`BuiltinScalarFunction` infrastructure, so I'm fine with considering that as
out of scope for now.
[1]:
https://www.postgresql.org/docs/13/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
