You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/02 09:45:32 UTC
[GitHub] [arrow] siddhantrao23 commented on a change in pull request #12285: ARROW-14631: [C++][Gandiva] Implement Nextday Function
siddhantrao23 commented on a change in pull request #12285:
URL: https://github.com/apache/arrow/pull/12285#discussion_r840345889
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -241,6 +241,44 @@ int getJanWeekOfYear(const EpochTimePoint& tp) {
return 52;
}
+static const char* WEEK[] = {"SUNDAY", "MONDAY", "TUESDAY", "WEDNESDAY",
+ "THURSDAY", "FRIDAY", "SATURDAY"};
+
+static const int WEEK_LEN[] = {6, 6, 7, 9, 8, 6, 8};
+
+#define NEXT_DAY_FUNC(TYPE) \
+ FORCE_INLINE \
+ gdv_date64 next_day_from_##TYPE(gdv_int64 context, gdv_##TYPE millis, const char* in, \
+ int32_t in_len) { \
+ EpochTimePoint tp(millis); \
+ const auto& day_without_hours_and_sec = tp.ClearTimeOfDay(); \
+ const auto& presentDate = extractDow_timestamp(tp.MillisSinceEpoch()); \
+ \
+ int dateSearch = 0; \
+ for (int n = 0; n < 7; n++) { \
+ if (is_substr_utf8_utf8(WEEK[n], WEEK_LEN[n], in, in_len)) { \
+ dateSearch = n + 1; \
+ break; \
+ } \
+ } \
+ if (dateSearch == 0) { \
+ gdv_fn_context_set_error_msg(context, "This entry not is one weekday valid"); \
Review comment:
Could the error message be "The weekday in this entry is invalid" instead
##########
File path: cpp/src/gandiva/precompiled/time.cc
##########
@@ -241,6 +241,44 @@ int getJanWeekOfYear(const EpochTimePoint& tp) {
return 52;
}
+static const char* WEEK[] = {"SUNDAY", "MONDAY", "TUESDAY", "WEDNESDAY",
+ "THURSDAY", "FRIDAY", "SATURDAY"};
+
+static const int WEEK_LEN[] = {6, 6, 7, 9, 8, 6, 8};
+
+#define NEXT_DAY_FUNC(TYPE) \
+ FORCE_INLINE \
+ gdv_date64 next_day_from_##TYPE(gdv_int64 context, gdv_##TYPE millis, const char* in, \
+ int32_t in_len) { \
+ EpochTimePoint tp(millis); \
+ const auto& day_without_hours_and_sec = tp.ClearTimeOfDay(); \
+ const auto& presentDate = extractDow_timestamp(tp.MillisSinceEpoch()); \
Review comment:
I would prefer if the variable names would be consistent in case, like camelCase or snake_case. Could you make all local variables the same case?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org