tustvold commented on PR #4291:
URL: https://github.com/apache/arrow-rs/pull/4291#issuecomment-1568502718
Thank you for this, I realise the issue I filed did you a disservice as it
was perhaps a touch underspecified.
I think this PR would be dramatically simplified if instead of implementing
decimal-based FixedPoint arithmetic as this PR does, instead did something
along the lines of
```
/// Chosen based on the number of decimal digits in 1 week in nanoseconds
const INTERVAL_PRECISION: u32 = 15;
struct IntervalComponent {
/// The whole interval component
integer: i64,
/// The non-integer component multiplied by 10^INTERVAL_PRECISION
frac: i64,
}
impl FromStr for IntervalComponent {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(match s.split_once('.') {
Some((integer, frac)) => {
if frac.len() > INTERVAL_PRECISION {
return Err(..)
}
let frac_s: i64 = frac.parse()?;
let frac = frac_s * 10.pow(INTERVAL_PRECISION - frac.len());
Self {
integer: integer.parse()?,
frac,
}
}
None => {
Self {
integer: s.parse()?,
frac: 0,
}
}
})
}
}
#[derive(Default)]
struct Interval {
months: i32,
days: i32,
nanos: i64,
}
impl Interval {
fn to_year_months(self) -> i32 {
self.months
}
fn to_day_time(self) -> Result<(i32, i32)> {
let days = self.months.checked_mul(30)?.checked_add(self.days);
let millis = self.nanos / 1_000_000;
Ok((days, millis))
}
fn to_month_day_nanos(self) -> (i32, i32, i64) {
(self.months, self.days, self.nanos)
}
/// Follow postgres logic
///
/// Fractional parts of units greater than months are rounded to be an
integer number of months
/// Fractional parts of units less than months are computed to be an
integer number of days and microseconds
fn add(&mut self, component: IntervalComponent, unit: IntervalUnit) ->
Result<()> {
match unit {
IntervalUnit::Second => {
let nanos = component.integer.checked_mul(1_000_000_000)?;
let nanos_frac = component.frac / 10.pow(INTERVAL_PRECISION
- 9);
self.nanos =
self.nanos.checked_add(nanos)?.checked_add(nanos_frac)?;
}
IntervalUnit::Week => {
self.days = self.days.checked_add(component.integer)?;
let nanos_frac = component.frac * 7 * 36 /
10.pow(INTERVAL_PRECISION - 7);
self.nanos = self.nanos.checked_add(nanos_frac)?;
}
IntervalUnit::Day => {
self.days = self.days.checked_add(component.integer)?;
let nanos_frac = component.frac * 36 /
10.pow(INTERVAL_PRECISION - 7);
self.nanos = self.nanos.checked_add(nanos_frac)?;
}
IntervalUnit::Century => {
let month_frac = component.frac_digits * 100 * 12 /
10.pow(component.frac_digits);
let months = component.integer.checked_mul(12)?;
self.months =
self.months.checked_add(months)?.checked_add(month_frac)?;
}
}
}
}
```
This is not only less code but avoids expensive integer division and modulus
operations, which are by far the slowest arithmetic operations on modern
processors, with LLVM automatically converting division by a constant to a
fixed point multiplication. It also allows eliding some overflow checks, as the
fractional component must be less than `10^INTERVAL_PRECISION`
--
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]
