`PrimitiveArray::validate_decimal_precision` rejects every decimal array with a negative scale

Open Beginner friendly
#12,095 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
rust
Domain
backend

Research direction

Read arrow-array/src/array/primitive_array.rs around PrimitiveArray::validate_decimal_precision, then compare its scale guard with validate_decimal_precision_and_scale in arrow-array/src/types.rs. Add a regression test for a negative-scale Decimal128Array such as the issue’s precision 5, scale -2 case; done means that value validates successfully while existing positive-scale checks still pass.

Written by the indexing model from the issue text.

Description

Describe the bug

PrimitiveArray::validate_decimal_precision returns Decimal precision {precision} is less than scale {scale} for any array whose scale is negative, whatever its precision and values. Its guard compares precision < self.scale() as u8, and a negative scale cast to u8 wraps, so -2 becomes 254.

https://github.com/apache/arrow-rs/blob/cc58320c2da359b757814db5a9651397d829b601/arrow-array/src/array/primitive_array.rs#L1708-L1714

The type-level check validate_decimal_precision_and_scale guards the same comparison with scale > 0:

https://github.com/apache/arrow-rs/blob/cc58320c2da359b757814db5a9651397d829b601/arrow-array/src/types.rs#L1465-L1469

The guard came in with #8396, first released in 57.0.0.

To Reproduce

On main at cc58320c2d:

use arrow_array::Decimal128Array;

fn main() {
    // 1 at scale -2 is 100, which fits in precision 5
    let a = Decimal128Array::from(vec![1])
        .with_precision_and_scale(5, -2)
        .unwrap();
    println!("{:?}", a.validate_decimal_precision(5));
    // Err(InvalidArgumentError("Decimal precision 5 is less than scale -2"))

    // The same value at scale 2 passes
    let b = Decimal128Array::from(vec![1])
        .with_precision_and_scale(5, 2)
        .unwrap();
    println!("{:?}", b.validate_decimal_precision(5));
    // Ok(())
}

Expected behavior

Ok(()) for the first array, since its value fits precision 5. A guard of self.scale() > 0 && precision < self.scale() as u8 would match validate_decimal_precision_and_scale.

Additional context

Property-based tests found it on a generated Decimal128(5, -2) array. Checking each value with DecimalType::validate_decimal_precision works around it.

Dominant language
Rust
Stars
3.7k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
314

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/arrow-rs

All issues in apache/arrow-rs

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.