Support negative scale decimals in decimal-to-integer conversions

[shifluxxc]

Which issue does this PR close?

• part of Allow log/pow on negative scale decimals #19250

Rationale for this change

Decimal values with negative scale represent multiplication by a power of ten, but the decimal-to-integer conversion helpers previously rejected such values. This caused valid negative-scale decimals to fail during evaluation.

What changes are included in this PR?

Update decimal32_to_i32, decimal64_to_i64, and decimal128_to_i128 to correctly handle negative scales by multiplying

Are these changes tested?

Yes. Existing unit tests were updated and new cases were added to validate correct behavio

[shifluxxc]

@alamb can you review the changes please .

[alamb]

Thank you for this PR @shifluxxc

I think the code looks good but I found the current test coverage in the .slt confusing

I think we need tests that try different types of decimals (including with negative scale), perhaps something like this (also adding Decimal32(9,-2), etc)? Though I don't fully comprehend the issue yet

# Test converting decimals to integers

statement ok
create table decimals as
select
  arrow_cast(column1, 'Decimal32(9,2)') as dec32
from values
    ('123.45'),
    ('-678.90'),
    ('0.00');

query R
select log(dec32, 10) from decimals;
----
0.478127782968
NaN
0

[alamb]

these tests pass on main as well (so I don't think they cover the intended usecase)

[shifluxxc]

fn log_decimal32(value: i32, scale: i8, base: f64) -> Result<f64, ArrowError> {
    if !base.is_finite() || base.trunc() != base {
        return Err(ArrowError::ComputeError(format!(
            "Log cannot use non-integer base: {base}"
        )));
    }
    if (base as u32) < 2 {
        return Err(ArrowError::ComputeError(format!(
            "Log base must be greater than 1: {base}"
        )));
    }

    // Match f64::log behaviour
    if value <= 0 {
        return Ok(f64::NAN);
    }

    if scale < 0 {
        let actual_value = (value as f64) * 10.0_f64.powi(-(scale as i32));
        Ok(actual_value.log(base))
    } else {
        let unscaled_value = decimal32_to_i32(value, scale)?;
        if unscaled_value <= 0 {
            return Ok(f64::NAN);
        }
        let log_value: u32 = unscaled_value.ilog(base as i32);
        Ok(log_value as f64)
    }
}

log_decimal32 currently bypasses decimal32_to_i32 when the scale is negative, instead using a manual floating-point calculation, and is not used elsewhere except for log, so how should i test it ?

[Jefffrey]

fn log_decimal32(value: i32, scale: i8, base: f64) -> Result<f64, ArrowError> {
    if !base.is_finite() || base.trunc() != base {
        return Err(ArrowError::ComputeError(format!(
            "Log cannot use non-integer base: {base}"
        )));
    }
    if (base as u32) < 2 {
        return Err(ArrowError::ComputeError(format!(
            "Log base must be greater than 1: {base}"
        )));
    }

    // Match f64::log behaviour
    if value <= 0 {
        return Ok(f64::NAN);
    }

    if scale < 0 {
        let actual_value = (value as f64) * 10.0_f64.powi(-(scale as i32));
        Ok(actual_value.log(base))
    } else {
        let unscaled_value = decimal32_to_i32(value, scale)?;
        if unscaled_value <= 0 {
            return Ok(f64::NAN);
        }
        let log_value: u32 = unscaled_value.ilog(base as i32);
        Ok(log_value as f64)
    }
}

log_decimal32 currently bypasses decimal32_to_i32 when the scale is negative, instead using a manual floating-point calculation, and is not used elsewhere except for log, so how should i test it ?

If the callers guard against passing in negative scale then I think we won't need to change the existing functions. Thanks for checking this, I'll close the origina issue.

[alamb]

Thanks @Jefffrey and @shifluxxc -- I am sorry I didn't better understand the issue