feat: Add Spark months_between function #12110
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
leoluan2009
force-pushed
the
months_between
branch
2 times, most recently
from
b65b99b to
ecd7e0f
Compare
| const auto monthsBetween2 = | ||
| [&](const StringView date1, const StringView date2, const bool roundOff) { | ||
| return evaluateOnce<double>( | ||
| "months_between(c0,c1)", |
There was a problem hiding this comment.
Is the test really covered? Here should be months_between(c0,c1,c2)
| struct MonthsBetweenFunction : InitSessionTimezone<TExec> { | ||
| VELOX_DEFINE_FUNCTION_TYPES(TExec); | ||
|
|
||
| FOLLY_ALWAYS_INLINE bool isEndDayOfMonth(const std::tm& dateTime) { |
There was a problem hiding this comment.
Please ,move the function to struct private scope
| FOLLY_ALWAYS_INLINE double monthsBetween( | ||
| const std::tm& datetime1, | ||
| const std::tm& datetime2, | ||
| const bool roundOff) { |
There was a problem hiding this comment.
Drop const when passing by value
| auto dayDiff = datetime1.tm_mday - datetime2.tm_mday; | ||
| auto hourDiff = datetime1.tm_hour - datetime2.tm_hour; | ||
| auto minuteDiff = datetime1.tm_min - datetime2.tm_min; | ||
| auto secondsDiff = dayDiff * 86400 + hourDiff * 3600 + minuteDiff * 60 + |
There was a problem hiding this comment.
Please use kSecondsInDay in TimeUtils.h
| auto diff = | ||
| monthDiff + static_cast<double>(secondsDiff) / (31 * 24 * 60 * 60); | ||
| if (roundOff) { | ||
| return std::round(diff * 100000000) / 100000000; |
There was a problem hiding this comment.
100000000 -> 100'000'000 for readable
| VELOX_DEFINE_FUNCTION_TYPES(TExec); | ||
|
|
||
| FOLLY_ALWAYS_INLINE bool isEndDayOfMonth(const std::tm& dateTime) { | ||
| auto year = dateTime.tm_year + 1900; |
There was a problem hiding this comment.
This is function getYear in TimeUtils.h
| (isEndDayOfMonth(datetime1) && isEndDayOfMonth(datetime2))) { | ||
| return static_cast<double>(monthDiff); | ||
| } | ||
| auto dayDiff = datetime1.tm_mday - datetime2.tm_mday; |
There was a problem hiding this comment.
Please refer to other functions in TimeUtils.h, you need month and secondsInDay
leoluan2009
force-pushed
the
months_between
branch
from
ecd7e0f to
8664fd8
Compare
leoluan2009
force-pushed
the
months_between
branch
from
8664fd8 to
3416eab
Compare
There was a problem hiding this comment.
@leoluan2009 Thanks. We had one PR #4859 which was closed due to stale. But would you like to check the comments inside it to see if all are addressed in this PR? Also, we could refer to its documentation.
Add Spark months_between function: https://github.com/apache/spark/blob/3b114af47ace4e834bc7c775d6e9e52b9056068b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala#L332