-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-16060: [C++] subtract_checked support for timestamp("s") and date32 #12774
Conversation
} else if (type == TimeUnit::SECOND) { | ||
ReplaceTemporalTypes(type, values); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without adding this we get:
'_error_or_value11.status()' failed with NotImplemented: Function 'subtract' has no kernel matching input types (array[timestamp[s]], array[date32[day]])
This only happens for (timestamp[s], day32) but not for (timestamp[ms,us,ns], day32) combinations. I'm not quite sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pitrou I'm sure I'm missing something trivial here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably because TimeUnit::SECOND is zero :) We should probably change the output type to wrap instd::optional
or something, or have bool CommonTemporalResolution(..., TimeUnit::type* out_unit)
CI issues seem unrelated. |
Benchmark runs are scheduled for baseline = bdb5ab8 and contender = 8eaa995. 8eaa995 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
…te32 This is to resolve [ARROW-16060](https://issues.apache.org/jira/browse/ARROW-16060). Closes apache#12774 from rok/ARROW-16060 Authored-by: Rok <rok@mihevc.org> Signed-off-by: David Li <li.davidm96@gmail.com>
This is to resolve ARROW-16060.