Skip to content
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

Jump to bottom

fix: Raise on oob decimal precision #17445

Merged
merged 1 commit into from
Jul 5, 2024
Merged

fix: Raise on oob decimal precision #17445

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
2 changes: 1 addition & 1 deletion crates/polars-core/src/chunked_array/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn cast_chunks(
let check_nulls = matches!(options, CastOptions::Strict);
let options = options.into();

let arrow_dtype = dtype.to_arrow(true);
let arrow_dtype = dtype.try_to_arrow(true)?;
chunks
.iter()
.map(|arr| {
Expand Down
15 changes: 10 additions & 5 deletions crates/polars-core/src/datatypes/dtype.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub enum DataType {
Float64,
/// Fixed point decimal type optional precision and non-negative scale.
/// This is backed by a signed 128-bit integer which allows for up to 38 significant digits.
/// Meaning max precision is 38.
#[cfg(feature = "dtype-decimal")]
Decimal(Option<usize>, Option<usize>), // precision/scale; scale being None means "infer"
/// String data
Expand Down Expand Up @@ -542,11 +543,15 @@ impl DataType {
Float32 => Ok(ArrowDataType::Float32),
Float64 => Ok(ArrowDataType::Float64),
#[cfg(feature = "dtype-decimal")]
// note: what else can we do here other than setting precision to 38?..
Decimal(precision, scale) => Ok(ArrowDataType::Decimal(
(*precision).unwrap_or(38),
scale.unwrap_or(0), // and what else can we do here?
)),
Decimal(precision, scale) => {
ritchie46 marked this conversation as resolved.
Show resolved Hide resolved
Copy link

Choose a reason for hiding this comment

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

PyArrow supports decimal256 too, is there any reason this should error as opposed to outputting a decimal256 array?

Copy link
Member Author

@ritchie46 ritchie46 Jul 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We don't support Decimal256. It turned out the precision given wasn't supported by Polars, so we raise earlier. Therefore we would also never need to export to Decimal256.

Copy link

Choose a reason for hiding this comment

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

Ah, ok

let precision = (*precision).unwrap_or(38);
polars_ensure!(precision <= 38 && precision > 0, InvalidOperation: "decimal precision should be <= 38 & >= 1");

Ok(ArrowDataType::Decimal(
precision,
scale.unwrap_or(0), // and what else can we do here?
))
},
String => {
let dt = if pl_flavor {
ArrowDataType::Utf8View
Expand Down
7 changes: 7 additions & 0 deletions py-polars/tests/unit/datatypes/test_decimal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,10 @@ def test_decimal_supertype() -> None:
pl.col("column_0").cast(pl.Decimal(scale=6)) * 1
)
assert q.collect().dtypes[0].is_decimal()


def test_decimal_raise_oob_precision() -> None:
df = pl.DataFrame({"a": [1.0]})
# max precision is 38.
with pytest.raises(pl.exceptions.InvalidOperationError):
df.select(b=pl.col("a").cast(pl.Decimal(76, 38)))
Loading