Skip to content

Build precision 19 decimal scalars at i128 #10903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
connortsui20 merged 1 commit into from
Oct 28, 2026
+52 −43
Merged

Build precision 19 decimal scalars at i128 #10903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 39 additions & 27 deletions vortex-jni/src/expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use vortex::VortexSessionDefault;
use vortex::dtype::BigCast;
use vortex::dtype::DType;
use vortex::dtype::DecimalDType;
use vortex::dtype::DecimalType;
use vortex::dtype::FieldName;
use vortex::dtype::FieldNames;
use vortex::dtype::MapDType;
Expand Down Expand Up @@ -576,7 +577,7 @@ pub extern "system" fn Java_dev_vortex_jni_NativeExpression_literalDecimal(
}

/// Decode a two's-complement big-endian byte array (Java `BigInteger.toByteArray()` format)
/// into the smallest [`DecimalValue`] variant that can hold the precision.
/// into the [`DecimalValue`] variant of the canonical width of the precision.
fn decimal_value_from_be_bytes(
bytes: &[u8],
dtype: &DecimalDType,
Expand All @@ -585,32 +586,17 @@ fn decimal_value_from_be_bytes(
throw_runtime!("decimal unscaled value must have at least one byte");
}
let value = i256_from_twos_complement_be(bytes);
// Pick the narrowest backing integer that fits the dtype's precision.
let required_bits = dtype.required_bit_width();
if required_bits <= 8 {
let v =
BigCast::from(value).ok_or_else(|| vortex_err!("decimal value does not fit in i8"))?;
Ok(DecimalValue::I8(v))
} else if required_bits <= 16 {
let v =
BigCast::from(value).ok_or_else(|| vortex_err!("decimal value does not fit in i16"))?;
Ok(DecimalValue::I16(v))
} else if required_bits <= 32 {
let v =
BigCast::from(value).ok_or_else(|| vortex_err!("decimal value does not fit in i32"))?;
Ok(DecimalValue::I32(v))
} else if required_bits <= 64 {
let v =
BigCast::from(value).ok_or_else(|| vortex_err!("decimal value does not fit in i64"))?;
Ok(DecimalValue::I64(v))
} else if required_bits <= 128 {
let v = value
.maybe_i128()
.ok_or_else(|| vortex_err!("decimal value does not fit in i128"))?;
Ok(DecimalValue::I128(v))
} else {
Ok(DecimalValue::I256(value))
}

let values_type = dtype.canonical_values_type();
let overflow = || vortex_err!("decimal value does not fit in {values_type}");
Ok(match values_type {
DecimalType::I8 => DecimalValue::I8(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I16 => DecimalValue::I16(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I32 => DecimalValue::I32(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I64 => DecimalValue::I64(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I128 => DecimalValue::I128(value.maybe_i128().ok_or_else(overflow)?),
DecimalType::I256 => DecimalValue::I256(value),
})
}

/// Sign-extend a two's-complement big-endian byte slice into an `i256`.
Expand Down Expand Up @@ -1063,3 +1049,29 @@ pub extern "system" fn Java_dev_vortex_jni_NativeExpression_literalNull(
Ok(into_raw(lit(Scalar::null(dtype))))
})
}

#[cfg(test)]
mod tests {
use rstest::rstest;
use vortex::dtype::DecimalDType;
use vortex::scalar::DecimalValue;

use super::decimal_value_from_be_bytes;
use crate::errors::JNIError;

/// Precision 19 holds values above `i64::MAX`, so the value must use the canonical `i128`.
#[rstest]
#[case::max(9_999_999_999_999_999_999)]
#[case::min(-9_999_999_999_999_999_999)]
fn decimal_value_at_precision_19_uses_i128(#[case] unscaled: i128) -> Result<(), JNIError> {
let dtype = DecimalDType::try_new(19, 0)?;

let value = decimal_value_from_be_bytes(&unscaled.to_be_bytes(), &dtype)?;

assert!(
matches!(value, DecimalValue::I128(v) if v == unscaled),
"expected DecimalValue::I128({unscaled}), got {value:?}"
);
Ok(())
}
}
27 changes: 11 additions & 16 deletions vortex-python/src/scalar/factory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use pyo3::types::PyString;
use vortex::dtype::BigCast;
use vortex::dtype::DType;
use vortex::dtype::DecimalDType;
use vortex::dtype::DecimalType;
use vortex::dtype::FieldName;
use vortex::dtype::FieldNames;
use vortex::dtype::MAX_PRECISION;
Expand Down Expand Up @@ -603,29 +604,23 @@ fn decimal_scalar(value: &Bound<'_, PyAny>, hint: Option<&DecimalDType>) -> PyVo

Ok(Scalar::try_new(
DType::Decimal(decimal_dtype, Nullability::NonNullable),
Some(ScalarValue::from(narrowest_decimal_value(
Some(ScalarValue::from(canonical_decimal_value(
unscaled,
&decimal_dtype,
)?)),
)?)
}

/// The narrowest [`DecimalValue`] variant able to hold every value of `dtype`'s precision.
fn narrowest_decimal_value(value: i256, dtype: &DecimalDType) -> PyResult<DecimalValue> {
/// The [`DecimalValue`] variant of the canonical width of `dtype`'s precision.
fn canonical_decimal_value(value: i256, dtype: &DecimalDType) -> PyResult<DecimalValue> {
Comment on lines +614 to +615

@connortsui20 connortsui20 Oct 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might blow things up but I think that is fine because it is a scalar, not an array?

let overflow = || PyValueError::new_err(format!("Decimal value {value} overflows {dtype}"));
let required_bits = dtype.required_bit_width();
Ok(if required_bits <= 8 {
DecimalValue::I8(BigCast::from(value).ok_or_else(overflow)?)
} else if required_bits <= 16 {
DecimalValue::I16(BigCast::from(value).ok_or_else(overflow)?)
} else if required_bits <= 32 {
DecimalValue::I32(BigCast::from(value).ok_or_else(overflow)?)
} else if required_bits <= 64 {
DecimalValue::I64(BigCast::from(value).ok_or_else(overflow)?)
} else if required_bits <= 128 {
DecimalValue::I128(value.maybe_i128().ok_or_else(overflow)?)
} else {
DecimalValue::I256(value)
Ok(match dtype.canonical_values_type() {
DecimalType::I8 => DecimalValue::I8(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I16 => DecimalValue::I16(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I32 => DecimalValue::I32(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I64 => DecimalValue::I64(BigCast::from(value).ok_or_else(overflow)?),
DecimalType::I128 => DecimalValue::I128(value.maybe_i128().ok_or_else(overflow)?),
DecimalType::I256 => DecimalValue::I256(value),
})
}

Expand Down
2 changes: 2 additions & 0 deletions vortex-python/test/test_scalar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ def test_timestamp_scalar_rejects_precision_loss() -> None:
("-0.001", 3, 3),
("12E+2", 4, 0),
("0", 1, 0),
("9" * 19, 19, 0),
("-" + "9" * 19, 19, 0),
("9" * 38, 38, 0),
],
)
Expand Down
Loading