viirya commented on code in PR #1914: URL: https://github.com/apache/arrow-rs/pull/1914#discussion_r901905221
########## arrow/src/util/decimal.rs: ########## @@ -17,109 +17,187 @@ //! Decimal related utils +use crate::error::{ArrowError, Result}; +use num::bigint::BigInt; use std::cmp::Ordering; +pub trait BasicDecimal: PartialOrd + Ord + PartialEq + Eq { + /// The bit-width of the internal representation. + const BIT_WIDTH: usize; + + /// Tries to create a decimal value from precision, scale and bytes. + /// If the length of bytes isn't same as the bit width of this decimal, + /// returning an error. The bytes should be stored in little-endian order. + /// + /// Safety: + /// This method doesn't validate if the decimal value represented by the bytes + /// can be fitted into the specified precision. + fn try_new_from_bytes(precision: usize, scale: usize, bytes: &[u8]) -> Result<Self> + where + Self: Sized, + { + if bytes.len() == Self::BIT_WIDTH / 8 { + Ok(Self::new(precision, scale, bytes)) + } else { + Err(ArrowError::InvalidArgumentError(format!( + "Input to Decimal{} must be {} bytes", + Self::BIT_WIDTH, + Self::BIT_WIDTH / 8 + ))) + } + } + + /// Creates a decimal value from precision, scale, and bytes. + /// + /// Safety: + /// This method doesn't check if the length of bytes is compatible with this decimal. + /// Use `try_new_from_bytes` for safe constructor. + fn new(precision: usize, scale: usize, bytes: &[u8]) -> Self; + + /// Returns the raw bytes of the integer representation of the decimal. + fn raw_value(&self) -> &[u8]; + + /// Returns the precision of the decimal. + fn precision(&self) -> usize; + + /// Returns the scale of the decimal. + fn scale(&self) -> usize; + + /// Returns the string representation of the decimal. + fn as_string(&self) -> String { + let raw_bytes = self.raw_value(); + let integer = BigInt::from_signed_bytes_le(raw_bytes); + let value_str = integer.to_string(); + + if self.scale() == 0 { + value_str + } else { + let (sign, rest) = + value_str.split_at(if integer >= BigInt::from(0) { 0 } else { 1 }); + + if rest.len() > self.scale() { + // Decimal separator is in the middle of the string + let (whole, decimal) = value_str.split_at(value_str.len() - self.scale()); + format!("{}.{}", whole, decimal) + } else { + // String has to be padded + format!("{}0.{:0>width$}", sign, rest, width = self.scale()) + } + } + } +} + /// Represents a decimal value with precision and scale. -/// The decimal value is represented by a signed 128-bit integer. +/// The decimal value could represented by a signed 128-bit integer. #[derive(Debug)] pub struct Decimal128 { #[allow(dead_code)] precision: usize, scale: usize, - value: i128, + value: [u8; 16], } -impl PartialOrd for Decimal128 { - fn partial_cmp(&self, other: &Self) -> Option<Ordering> { - assert_eq!( - self.scale, other.scale, - "Cannot compare two Decimal128 with different scale: {}, {}", - self.scale, other.scale - ); - self.value.partial_cmp(&other.value) +impl Decimal128 { + /// Creates `Decimal128` from an `i128` value. + pub fn new_from_i128(precision: usize, scale: usize, value: i128) -> Self { Review Comment: It is mostly used in test code internally. Another option is to not expose it, maybe we can make it crate public. -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org