akravchukdremio commented on code in PR #51266:
URL: https://github.com/apache/arrow/pull/51266#discussion_r4025700246
##########
cpp/src/arrow/scalar.cc:
##########
@@ -1191,41 +1193,45 @@ constexpr int64_t kMillisecondsInDay = 86400000;
// date to date
template <typename To>
-enable_if_t<std::is_same<To, Date64Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date64Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const Date32Scalar& from, std::shared_ptr<DataType> to_type) {
return std::make_shared<Date64Scalar>(from.value * kMillisecondsInDay,
std::move(to_type));
}
template <typename To>
-enable_if_t<std::is_same<To, Date32Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date32Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const Date64Scalar& from, std::shared_ptr<DataType> to_type) {
return std::make_shared<Date32Scalar>(
static_cast<int32_t>(from.value / kMillisecondsInDay),
std::move(to_type));
}
// timestamp to date
template <typename To>
-enable_if_t<std::is_same<To, Date64Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date64Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const TimestampScalar& from, std::shared_ptr<DataType> to_type) {
ARROW_ASSIGN_OR_RAISE(
auto millis,
util::ConvertTimestampValue(from.type, timestamp(TimeUnit::MILLI),
from.value));
- return std::make_shared<Date64Scalar>(millis - millis % kMillisecondsInDay,
+ const auto days_since_epoch =
+
internal::chrono::floor<internal::chrono::days>(std::chrono::milliseconds{millis});
+ return std::make_shared<Date64Scalar>(days_since_epoch.count() *
kMillisecondsInDay,
Review Comment:
Agreed on skipping bounds checks here. If a real use case/issue shows up, we
can follow up it separately
--
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]
