ARROW-14092: [C++] subtract(date, duration) -> timestamp kernel #12136
Conversation
lidavidm
left a comment
There was a problem hiding this comment.
As discussed on other PRs we probably need to add more implicit casts for this (e.g. date32 - duration[ns] => timestamp[ns] - duration[ns] perhaps)
docs/source/cpp/compute.rst
Outdated
There was a problem hiding this comment.
Should we add a note about temporal casting here?
There was a problem hiding this comment.
Now that I think about it, this is kind of weird to use CommonTemporal since there isn't actually a "common" type between date32 and duration (for instance) and what we actually care about here is the common unit - maybe it's worth splitting that off separately? Since future/other uses of CommonTemporal might be 'surprised' to find that CommonTemporal(date, duration) => date instead of nullptr.
There was a problem hiding this comment.
How about renaming CommonTemporal to CommonResolution and returning finest time unit resolution?
There was a problem hiding this comment.
CommonTemporal is used elsewhere, but having CommonResolution alongside it makes sense.
There was a problem hiding this comment.
Added CommonTemporalResolution and refactored ReplaceTemporalTypes. This PR should hopefully cover all temporal casts needed for ARROW-11901.
bc71a08 to
7e7b417
Compare
|
@lidavidm rebased one more time and changed back to |
|
Sorry for the delay - time to rebase the other PRs now :) (feel free to ping me on anything that needs looking at) |
|
Benchmark runs are scheduled for baseline = 9e69ccf and contender = c4eb8dc. c4eb8dc is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
https://issues.apache.org/jira/browse/ARROW-14092