vortex-data / vortex / 16840805129

88%

push

github

fix: incorrect decimal scalar nbytes (#4176)

# Summary

Successfully improved test coverage for the vortex-scalar crate by
analyzing a coverage report showing 11 files with coverage below 85% and
systematically adding comprehensive tests. The work increased the test
count from 294 to 419 tests (125 new tests).

## Key Accomplishments

  1. Critical Bug Fix

- Fixed decimal nbytes() logic bug (vortex-scalar/src/lib.rs#L192): The
comparison was inverted - changed from >=
  to <= when checking DECIMAL128_MAX_PRECISION
- This bug would have caused i128 decimals to report 32 bytes instead of
16 bytes, and i256 decimals to report 16
  bytes instead of 32 bytes

  2. Documentation Improvements

- Added comprehensive F16 coercion documentation explaining the
bit-pattern interpretation behavior
- Documented PartialOrd behavior with clear examples of when comparisons
return None vs Some
- Improved error messages for type casting failures to be more
descriptive

  3. Testing Patterns Established

  - Comprehensive boundary testing for numeric types
  - Null handling across all scalar types
  - Type casting with overflow detection
  - Error case validation
  - Partial ordering behavior verification

## Test Categories Added

1. Type Conversions: Comprehensive casting between decimal, primitive,
and other types
  2. Boundary Conditions: Testing limits of each numeric type
  3. Null Handling: Ensuring proper null propagation and handling
  4. Error Cases: Validating appropriate errors for invalid operations
  5. Memory Management: Testing nbytes() calculations for all types
  6. Temporal Extensions: Arrow FFI conversion for time/date types

## Bug Fixed

```rust
  // BEFORE (BUG):
  if dt.precision() >= DECIMAL128_MAX_PRECISION {
      size_of::<i128>()
  } else {
      size_of::<i256>()
  }
```

```rust
  // AFTER (FIXED):
  if dt.precision() <= DECIMAL128_MAX_PRECISION {
      size_of::<i128>()
  } else {
      size_of::<i256>()
  }
```

## Impact

The improveme... (continued)

1755 of 1768 new or added lines in 12 files covered. (99.26%)

53520 of 62064 relevant lines covered (86.23%)

546012.47 hits per line