[GitHub] [arrow] lidavidm commented on a diff in pull request #12657: ARROW-14821: [C++][R] Implement bindings for lubridate's floor_date, ceiling_date, and round_date

[GitBox <gi...@apache.org>]

lidavidm commented on code in PR #12657:
URL: https://github.com/apache/arrow/pull/12657#discussion_r860201453


##########
cpp/src/arrow/compute/api_scalar.h:
##########
@@ -117,6 +118,13 @@ class ARROW_EXPORT RoundTemporalOptions : public FunctionOptions {
   CalendarUnit unit;
   /// What day does the week start with (Monday=true, Sunday=false)
   bool week_starts_monday;
+  /// Times exactly on unit multiple boundary will be rounded one unit multiple up.
+  /// This applies for ceiling only.

Review Comment:
   So taking the day-ceiling of `2022-01-01 00:00:00` is normally a no-op, but will return `2022-01-02 00:00:00` with this enabled?



##########
cpp/src/arrow/compute/kernels/scalar_temporal_test.cc:
##########
@@ -2229,6 +2229,367 @@ TEST_F(ScalarTemporalTest, TestCeilTemporal) {
   CheckScalarUnary(op, unit, times, unit, ceil_15_years, &round_to_15_years);
 }
 
+TEST_F(ScalarTemporalTest, TestCeilTemporalChangeOnBoundary) {

Review Comment:
   nit, but the parameter was renamed while the tests weren't



-- 
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