Copilot commented on code in PR #51266:
URL: https://github.com/apache/arrow/pull/51266#discussion_r4012176874
##########
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:
For a valid millisecond timestamp of `INT64_MIN`, `floor` produces a day
count below the representable `Date64` range; multiplying it by
`kMillisecondsInDay` here overflows signed `int64_t` before the scalar is
constructed. Since `CastTo` returns a `Result`, check the day-count bounds (or
use checked multiplication) and return an error for this boundary case, with a
regression test.
--
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]
