zeroshade commented on code in PR #1127:
URL: https://github.com/apache/arrow-go/pull/1127#discussion_r3896328390
##########
arrow/compute/internal/kernels/rounding.go:
##########
@@ -1005,145 +1280,592 @@ func roundToMultipleInt64(value, multiple int64, mode
RoundMode, strictCeil bool
// a remainder of 1 when rounding to multiples of 3 is closer
to 0
// than to 3, so it must not be treated as a tie.
if absRemainder < half || (multiple%2 != 0 && absRemainder ==
half) {
- return quotient * multiple
+ return checkedMulInt64(quotient, multiple)
} else if absRemainder > half {
if remainder > 0 {
- return (quotient + 1) * multiple
+ quotient, err := checkedAddInt64(quotient, 1)
+ if err != nil {
+ return 0, err
+ }
+ return checkedMulInt64(quotient, multiple)
+ }
+ quotient, err := checkedSubInt64(quotient, 1)
+ if err != nil {
+ return 0, err
}
- return (quotient - 1) * multiple
+ return checkedMulInt64(quotient, multiple)
} else {
// Exactly on the halfway point
switch mode {
case HalfDown:
if remainder > 0 {
- return quotient * multiple
+ return checkedMulInt64(quotient,
multiple)
+ }
+ quotient, err := checkedSubInt64(quotient, 1)
+ if err != nil {
+ return 0, err
}
- return (quotient - 1) * multiple
+ return checkedMulInt64(quotient, multiple)
case HalfUp:
if remainder > 0 {
- return (quotient + 1) * multiple
+ quotient, err :=
checkedAddInt64(quotient, 1)
+ if err != nil {
+ return 0, err
+ }
+ return checkedMulInt64(quotient,
multiple)
}
- return quotient * multiple
+ return checkedMulInt64(quotient, multiple)
case HalfToEven:
if quotient%2 == 0 {
- return quotient * multiple
+ return checkedMulInt64(quotient,
multiple)
}
if remainder > 0 {
- return (quotient + 1) * multiple
+ quotient, err :=
checkedAddInt64(quotient, 1)
+ if err != nil {
+ return 0, err
+ }
+ return checkedMulInt64(quotient,
multiple)
+ }
+ quotient, err := checkedSubInt64(quotient, 1)
+ if err != nil {
+ return 0, err
}
- return (quotient - 1) * multiple
+ return checkedMulInt64(quotient, multiple)
}
}
}
- return quotient * multiple
+ return checkedMulInt64(quotient, multiple)
+}
+
+// halfRoundPeriod performs half-rounding by finding the midpoint between
period start and end.
+// It does not use time.Time.Sub or time.Time.Unix because those methods
cannot represent
+// sufficiently large calendar periods and timestamps.
+func halfRoundPeriod(t, periodStart, periodEnd time.Time) (time.Time, error) {
+ startDays, startNanos, err := calendarTimeParts(periodStart)
+ if err != nil {
+ return time.Time{}, err
+ }
+ endDays, endNanos, err := calendarTimeParts(periodEnd)
+ if err != nil {
+ return time.Time{}, err
+ }
+ if endDays < startDays || (endDays == startDays && endNanos <
startNanos) {
+ return time.Time{}, overflowError()
+ }
+ deltaDays, err := checkedSubInt64(endDays, startDays)
+ if err != nil {
+ return time.Time{}, err
+ }
+ deltaNanos, err := checkedSubInt64(endNanos, startNanos)
+ if err != nil {
+ return time.Time{}, err
+ }
+ if deltaNanos < 0 {
+ deltaDays, err = checkedSubInt64(deltaDays, 1)
+ if err != nil {
+ return time.Time{}, err
+ }
+ deltaNanos += nanosPerDay
+ }
+
+ tDays, tNanos, err := calendarTimeParts(t)
+ if err != nil {
+ return time.Time{}, err
+ }
+ relativeDays, err := checkedSubInt64(tDays, startDays)
+ if err != nil {
+ return time.Time{}, err
+ }
+ relativeNanos, err := checkedSubInt64(tNanos, startNanos)
+ if err != nil {
+ return time.Time{}, err
+ }
+ if relativeNanos < 0 {
+ relativeDays, err = checkedSubInt64(relativeDays, 1)
+ if err != nil {
+ return time.Time{}, err
+ }
+ relativeNanos += nanosPerDay
+ }
+
+ // Compare 2*(t-periodStart) with periodEnd-periodStart without ever
+ // converting the whole interval to a time.Duration.
+ doubledDays, err := checkedMulInt64(relativeDays, 2)
+ if err != nil {
+ return time.Time{}, err
+ }
+ doubledNanos, err := checkedMulInt64(relativeNanos, 2)
+ if err != nil {
+ return time.Time{}, err
+ }
+ if doubledNanos >= nanosPerDay {
+ doubledDays, err = checkedAddInt64(doubledDays, 1)
+ if err != nil {
+ return time.Time{}, err
+ }
+ doubledNanos -= nanosPerDay
+ }
+
+ if doubledDays < deltaDays || (doubledDays == deltaDays && doubledNanos
< deltaNanos) {
+ return periodStart, nil
+ }
+ return periodEnd, nil
+}
+
+const nanosPerDay = int64(24 * time.Hour)
+
+// calendarTimeParts returns a time instant as a day number and nanoseconds
into
+// the corresponding UTC day. Unlike Unix timestamps, this representation
+// remains valid for all time.Time values supported by the time package.
+func calendarTimeParts(value time.Time) (int64, int64, error) {
+ days, err := calendarDays(value)
+ if err != nil {
+ return 0, 0, err
+ }
+
+ seconds := int64(value.Hour())*3600 + int64(value.Minute())*60 +
int64(value.Second())
+ nanos, err := checkedMulInt64(seconds, int64(time.Second))
+ if err != nil {
+ return 0, 0, err
+ }
+ nanos, err = checkedAddInt64(nanos, int64(value.Nanosecond()))
+ if err != nil {
+ return 0, 0, err
+ }
+ _, offset := value.Zone()
+ offsetNanos, err := checkedMulInt64(int64(offset), int64(time.Second))
+ if err != nil {
+ return 0, 0, err
+ }
+ nanos, err = checkedSubInt64(nanos, offsetNanos)
+ if err != nil {
+ return 0, 0, err
+ }
+
+ if nanos < 0 {
+ days, err = checkedSubInt64(days, 1)
+ if err != nil {
+ return 0, 0, err
+ }
+ nanos += nanosPerDay
+ } else if nanos >= nanosPerDay {
+ days, err = checkedAddInt64(days, 1)
+ if err != nil {
+ return 0, 0, err
+ }
+ nanos -= nanosPerDay
+ }
+
+ return days, nanos, nil
+}
+
+// calendarDays returns the proleptic Gregorian day number for a local date.
+// The calculation is based on civil calendar fields, so it does not saturate
+// at time.Duration or int64 Unix-second boundaries.
+func calendarDays(value time.Time) (int64, error) {
+ year := int64(value.Year())
+ month := int64(value.Month())
+ day := int64(value.Day())
+ if month <= 2 {
+ var err error
+ year, err = checkedSubInt64(year, 1)
+ if err != nil {
+ return 0, err
+ }
+ }
+
+ era := year / 400
+ if year < 0 && year%400 != 0 {
+ era--
+ }
+ eraYear, err := checkedMulInt64(era, 400)
+ if err != nil {
+ return 0, err
+ }
+ yearOfEra, err := checkedSubInt64(year, eraYear)
+ if err != nil {
+ return 0, err
+ }
+
+ monthOfMarch := month
+ if month > 2 {
+ monthOfMarch -= 3
+ } else {
+ monthOfMarch += 9
+ }
+ daysOfYear := (153*monthOfMarch+2)/5 + day - 1
+ daysOfEra := yearOfEra*365 + yearOfEra/4 - yearOfEra/100 + daysOfYear
+ days, err := checkedMulInt64(era, 146097)
+ if err != nil {
+ return 0, err
+ }
+ days, err = checkedAddInt64(days, daysOfEra)
+ if err != nil {
+ return 0, err
+ }
+ return checkedSubInt64(days, 719468)
+}
+
+func calendarDayDifference(left, right time.Time) (int64, error) {
+ leftDays, err := calendarDays(left)
+ if err != nil {
+ return 0, err
+ }
+ rightDays, err := calendarDays(right)
+ if err != nil {
+ return 0, err
+ }
+ return checkedSubInt64(leftDays, rightDays)
+}
+
+// floorCalendarIndex returns the start of the current calendar period as an
+// index in either months or quarters. Without a calendar origin, periods are
+// anchored at 1970-01-01. With one, multiple periods start at the beginning of
+// the current year.
+func floorCalendarIndex(year, offset, periodsPerYear, multiple int64,
calendarOrigin bool) (int64, error) {
+ yearIndex, err := checkedMulInt64(year, periodsPerYear)
+ if err != nil {
+ return 0, err
+ }
+ currentIndex, err := checkedAddInt64(yearIndex, offset)
+ if err != nil {
+ return 0, err
+ }
+
+ if calendarOrigin && multiple != 1 {
+ roundedOffset, err := checkedMulInt64(floorDivInt64(offset,
multiple), multiple)
+ if err != nil {
+ return 0, err
+ }
+ return checkedAddInt64(yearIndex, roundedOffset)
+ }
+
+ epochIndex, err := checkedMulInt64(1970, periodsPerYear)
+ if err != nil {
+ return 0, err
+ }
+ relative, err := checkedSubInt64(currentIndex, epochIndex)
+ if err != nil {
+ return 0, err
+ }
+ roundedRelative, err := checkedMulInt64(floorDivInt64(relative,
multiple), multiple)
+ if err != nil {
+ return 0, err
+ }
+ return checkedAddInt64(epochIndex, roundedRelative)
+}
+
+func calendarDateFromIndex(index, periodsPerYear, monthsPerPeriod int64, tz
*time.Location) (time.Time, error) {
+ year := floorDivInt64(index, periodsPerYear)
+ yearIndex, err := checkedMulInt64(year, periodsPerYear)
+ if err != nil {
+ return time.Time{}, err
+ }
+ offset, err := checkedSubInt64(index, yearIndex)
+ if err != nil {
+ return time.Time{}, err
+ }
+ monthOffset, err := checkedMulInt64(offset, monthsPerPeriod)
+ if err != nil {
+ return time.Time{}, err
+ }
+ month, err := checkedAddInt64(monthOffset, 1)
+ if err != nil {
+ return time.Time{}, err
+ }
+ return checkedCalendarDate(year, month, tz)
+}
+
+func floorCalendarDay(value time.Time, multiple int64, calendarOrigin bool, tz
*time.Location) (time.Time, error) {
+ day := time.Date(value.Year(), value.Month(), value.Day(), 0, 0, 0, 0,
tz)
+ if multiple == 1 {
+ return day, nil
+ }
+
+ origin := time.Date(1970, 1, 1, 0, 0, 0, 0, tz)
+ if calendarOrigin {
+ origin = time.Date(value.Year(), value.Month(), 1, 0, 0, 0, 0,
tz)
+ }
+ daysSinceOrigin, err := calendarDayDifference(day, origin)
+ if err != nil {
+ return time.Time{}, err
+ }
+ roundedDays, err := checkedMulInt64(floorDivInt64(daysSinceOrigin,
multiple), multiple)
+ if err != nil {
+ return time.Time{}, err
+ }
+ return checkedCalendarAddDays(origin, roundedDays)
}
-// halfRoundPeriod performs half-rounding by finding the midpoint between
period start and end
-func halfRoundPeriod(t, periodStart, periodEnd time.Time) time.Time {
- midPoint := periodStart.Add(periodEnd.Sub(periodStart) / 2)
- if t.Before(midPoint) {
- return periodStart
+func floorCalendarWeek(value time.Time, multiple int64, weekStartsMonday bool,
tz *time.Location) (time.Time, error) {
+ offsetDays := int64(4)
+ targetWeekday := time.Wednesday
+ if weekStartsMonday {
+ offsetDays = 3
+ targetWeekday = time.Thursday
+ }
+ shifted, err := checkedCalendarAddDays(value, offsetDays)
Review Comment:
Calendar-origin week rounding can return a floor later than its input. For
example, flooring `1078-06-14 04:11:08 UTC` to a two-week interval starting
Monday returns `1078-06-17 00:00:00 UTC`. The bucket is calculated from the
shifted value without restoring the original-week position. Please correct the
shift handling and add tests covering mid-week inputs, multiples greater than
one, both week-start modes, and `floor <= input <= ceil`.
--
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]
