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

Don't validate decimal precision in ArrayData (#2637) #2873

Merged
merged 2 commits into from Oct 14, 2022
Merged

Don't validate decimal precision in ArrayData (#2637) #2873

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c92945c
Don't validate decimal precision in ArrayData (#2637)
tustvold Oct 14, 2022
4e2687a
Format
tustvold Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 0 additions & 19 deletions arrow-data/src/data.rs
View file
Open in desktop
Expand Up @@ -18,9 +18,6 @@
//! Contains `ArrayData`, a generic representation of Arrow array data which encapsulates
//! common attributes and operations for Arrow array.

use crate::decimal::{
validate_decimal256_precision_with_lt_bytes, validate_decimal_precision,
};
use crate::{bit_iterator::BitSliceIterator, bitmap::Bitmap};
use arrow_buffer::{bit_util, ArrowNativeType, Buffer, MutableBuffer};
use arrow_schema::{ArrowError, DataType, IntervalUnit, UnionMode};
Expand Down Expand Up @@ -1004,22 +1001,6 @@ impl ArrayData {

pub fn validate_values(&self) -> Result<(), ArrowError> {
match &self.data_type {
DataType::Decimal128(p, _) => {
let values_buffer: &[i128] = self.typed_buffer(0, self.len)?;
for value in values_buffer {
validate_decimal_precision(*value, *p)?;
}
Ok(())
}
DataType::Decimal256(p, _) => {
let values = self.buffers()[0].as_slice();
for pos in 0..self.len() {
let offset = pos * 32;
let raw_bytes = &values[offset..offset + 32];
validate_decimal256_precision_with_lt_bytes(raw_bytes, *p)?;
}
Ok(())
}
DataType::Utf8 => self.validate_utf8::<i32>(),
DataType::LargeUtf8 => self.validate_utf8::<i64>(),
DataType::Binary => self.validate_offsets_full::<i32>(self.buffers[1].len()),
Expand Down
9 changes: 6 additions & 3 deletions arrow/tests/array_validation.rs
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ use arrow_data::ArrayData;
use arrow_schema::{DataType, Field, UnionMode};
use std::ptr::NonNull;
use std::sync::Arc;
use arrow_array::Decimal128Array;

#[test]
#[should_panic(
Expand Down Expand Up @@ -1038,7 +1039,6 @@ fn test_string_data_from_foreign() {
}

#[test]
#[cfg(not(feature = "force_validate"))]
fn test_decimal_full_validation() {
let values_builder = UInt8Builder::with_capacity(10);
let byte_width = 16;
Expand All @@ -1055,8 +1055,11 @@ fn test_decimal_full_validation() {
.len(fixed_size_array.len())
.add_buffer(fixed_size_array.data_ref().child_data()[0].buffers()[0].clone());
let array_data = unsafe { builder.build_unchecked() };
let validation_result = array_data.validate_full();
let error = validation_result.unwrap_err();
array_data.validate_full().unwrap();

let array = Decimal128Array::from(array_data);
let error = array.validate_decimal_precision(array.precision()).unwrap_err();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is now explicitly opt-in


assert_eq!(
"Invalid argument error: 123456 is too large to store in a Decimal128 of precision 5. Max is 99999",
error.to_string()
Expand Down