projjal commented on a change in pull request #10160:
URL: https://github.com/apache/arrow/pull/10160#discussion_r629859574
##########
File path: cpp/src/gandiva/precompiled/epoch_time_point.h
##########
@@ -19,6 +19,14 @@
// TODO(wesm): IR compilation does not have any include directories set
#include "../../arrow/vendored/datetime/date.h"
+#ifndef UTIL_FUNCTIONS
Review comment:
what use of this #ifndef?
##########
File path: cpp/src/gandiva/precompiled/timestamp_arithmetic.cc
##########
@@ -170,17 +194,41 @@ TIMESTAMP_DIFF(timestamp)
FORCE_INLINE \
gdv_##TYPE NAME##_##TYPE##_int64(gdv_##TYPE millis, gdv_int64 count) { \
return millis + TO_MILLIS * static_cast<gdv_##TYPE>(count); \
+ }
+
+
+#define ADD_TIMESTAMP_TO_INT32_MONTH_UNITS(TYPE, NAME, N_MONTHS)
\
+ FORCE_INLINE
\
+ gdv_##TYPE NAME##_##TYPE##_int32(gdv_##TYPE millis, gdv_int32 count) {
\
+ EpochTimePoint tp(millis);
\
+ return tp.AddMonths(static_cast<int>(count *
N_MONTHS)).MillisSinceEpoch(); \
+ }
+
+#define ADD_TIMESTAMP_TO_INT64_MONTH_UNITS(TYPE, NAME, N_MONTHS)
\
+ FORCE_INLINE
\
+ gdv_##TYPE NAME##_##TYPE##_int64(gdv_##TYPE millis, gdv_int64 count) {
\
+ EpochTimePoint tp(millis);
\
+ return tp.AddMonths(static_cast<int>(count *
N_MONTHS)).MillisSinceEpoch(); \
}
#define TIMESTAMP_ADD_INT32(TYPE) \
ADD_INT32_TO_TIMESTAMP_FIXED_UNITS(TYPE, timestampaddSecond, MILLIS_IN_SEC) \
+ ADD_TIMESTAMP_TO_INT32_FIXED_UNITS(TYPE, timestampaddSecond, MILLIS_IN_SEC) \
Review comment:
nit: add a macro ADD_TIMESTAMP_INT32 which calls ADD_TIMESTAMP_TO_INT32
and ADD_INT32_TO_TIMESTAMP
##########
File path: cpp/src/gandiva/precompiled/epoch_time_point.h
##########
@@ -65,12 +73,22 @@ class EpochTimePoint {
.time_since_epoch());
}
- EpochTimePoint AddMonths(int num_months) const {
- auto ymd = YearMonthDay() + arrow_vendored::date::months(num_months);
- return EpochTimePoint((arrow_vendored::date::sys_days{ymd} + // NOLINT
- TimeOfDay().to_duration())
- .time_since_epoch());
- }
+ EpochTimePoint AddMonths(int num_months) const {
+ auto ymd = YearMonthDay() + arrow_vendored::date::months(num_months);
+
+ EpochTimePoint tp = EpochTimePoint((arrow_vendored::date::sys_days{ymd}
+ // NOLINT
+ TimeOfDay().to_duration())
+ .time_since_epoch());
+
+ if(did_days_overflow(static_cast<int>(ymd.year()), static_cast<unsigned
int>(ymd.month()),
Review comment:
nit: better pass `date::year_month_day` as argument
##########
File path: cpp/src/gandiva/precompiled/timestamp_arithmetic.cc
##########
@@ -47,6 +47,28 @@ bool is_last_day_of_month(const EpochTimePoint& tp) {
return !is_leap_year(tp.TmYear());
}
+bool did_days_overflow(int year, int month, int days) {
+ int days_in_month[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
Review comment:
you can set a static days_in_month array
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
