wgtmac commented on code in PR #48247:
URL: https://github.com/apache/arrow/pull/48247#discussion_r2560637334
##########
cpp/src/parquet/column_writer.h:
##########
@@ -259,14 +259,15 @@ constexpr int64_t kJulianEpochOffsetDays =
INT64_C(2440588);
template <int64_t UnitPerDay, int64_t NanosecondsPerUnit>
inline void ArrowTimestampToImpalaTimestamp(const int64_t time, Int96*
impala_timestamp) {
- int64_t julian_days = (time / UnitPerDay) + kJulianEpochOffsetDays;
- (*impala_timestamp).value[2] = (uint32_t)julian_days;
+ int32_t julian_days = static_cast<int32_t>(time / UnitPerDay +
kJulianEpochOffsetDays) +
Review Comment:
I just did some research on how to adjust negative values and it seems that
following code looks more correct:
```cpp
int64_t q = time / UnitPerDay;
int64_t r = time % UnitPerDay;
if (r < 0) {
q -= 1;
r += UnitPerDay;
}
uint32_t julian_days = static_cast<uint32_t>(q + kJulianEpochOffsetDays);
impala_timestamp->value[2] = julian_days;
uint64_t last_day_units = static_cast<uint64_t>(r);
uint64_t last_day_nanos = last_day_units * NanosecondsPerUnit;
std::memcpy(impala_timestamp, &last_day_nanos, sizeof(uint64_t));
```
The main difference is when time is an exact multiple of UnitPerDay.
--
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]
