HaoYang670 commented on code in PR #2360:
URL: https://github.com/apache/arrow-rs/pull/2360#discussion_r944046416
##########
arrow/src/array/array_decimal.rs:
##########
@@ -332,15 +331,30 @@ impl Decimal128Array {
impl Decimal256Array {
/// Validates decimal values in this array can be properly interpreted
/// with the specified precision.
- pub fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
- if precision < self.precision {
- for v in self.iter().flatten() {
- validate_decimal256_precision(&v.to_big_int(), precision)?;
+ fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
Review Comment:
Personally, I prefer this function to be an independent `fn`, but not a
method of `Decimal256Array`,
because
1. DecimalArray has its own precision, providing another `precision` to its
method is somewhat weird.
2. This method is just for `Decimal256Array`, but not the generic decimal
array. Moving it out the `impl Decimal256Array` can make the code cleaner.
##########
arrow/src/array/array_decimal.rs:
##########
@@ -332,15 +331,30 @@ impl Decimal128Array {
impl Decimal256Array {
/// Validates decimal values in this array can be properly interpreted
/// with the specified precision.
- pub fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
- if precision < self.precision {
- for v in self.iter().flatten() {
- validate_decimal256_precision(&v.to_big_int(), precision)?;
+ fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
Review Comment:
Do we need this check if `precision >= self.precision`?
##########
arrow/src/array/array_decimal.rs:
##########
@@ -332,15 +331,30 @@ impl Decimal128Array {
impl Decimal256Array {
/// Validates decimal values in this array can be properly interpreted
/// with the specified precision.
- pub fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
- if precision < self.precision {
- for v in self.iter().flatten() {
- validate_decimal256_precision(&v.to_big_int(), precision)?;
+ fn validate_decimal_precision(&self, precision: usize) -> Result<()> {
+ let current_end = self.data.len();
+ let mut current: usize = 0;
+ let data = &self.data;
+
+ while current != current_end {
+ if self.is_null(current) {
+ current += 1;
+ continue;
+ } else {
+ let offset = current + data.offset();
+ current += 1;
+ let raw_val = unsafe {
Review Comment:
This could be extracted as a separate method `DecimalArray::raw_value`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
