liurenjie1024 commented on code in PR #665:
URL: https://github.com/apache/iceberg-rust/pull/665#discussion_r1875657680
##########
crates/iceberg/src/spec/values.rs:
##########
@@ -1012,6 +1053,33 @@ impl Datum {
}
}
+ /// Try to create a decimal literal from [`Decimal`] with precision.
+ ///
+ /// Example:
+ ///
+ /// ```rust
+ /// use iceberg::spec::Datum;
+ /// use rust_decimal::Decimal;
+ ///
+ /// let t = Datum::decimal_with_precision(Decimal::new(123, 2),
30).unwrap();
+ ///
+ /// assert_eq!(&format!("{t}"), "1.23");
+ /// ```
+ pub fn decimal_with_precision(value: impl Into<Decimal>, precision: u32)
-> Result<Self> {
+ let decimal = value.into();
+ let scale = decimal.scale();
+
+ let r#type = Type::decimal(precision, scale)?;
+ if let Type::Primitive(p) = r#type {
+ Ok(Self {
+ r#type: p,
+ literal: PrimitiveLiteral::Int128(decimal.mantissa()),
Review Comment:
This is incorrect, we need to validate that the precision is large enough to
hold this value. For example, if the input decimal is `123456789`, and the
required precision is 4, then we should throw an error here.
##########
crates/iceberg/src/spec/values.rs:
##########
@@ -3031,6 +3061,31 @@ mod tests {
check_avro_bytes_serde(bytes, Datum::string("iceberg"),
&PrimitiveType::String);
}
+ #[test]
+ fn avro_bytes_decimal() {
Review Comment:
> do you have an idea why we always pick up the max precision here?
IIRC, this is because `Decimal` is 128 bit and fits into max precision.
> Should we have an API like decimal_with_precision that allow users to
specify the precision they want?
Absolutely.
##########
crates/iceberg/src/spec/manifest.rs:
##########
@@ -1451,13 +1451,15 @@ mod _serde {
Ok(m)
}
- fn to_bytes_entry(v: impl IntoIterator<Item = (i32, Datum)>) ->
Vec<BytesEntry> {
- v.into_iter()
- .map(|e| BytesEntry {
- key: e.0,
- value: e.1.to_bytes(),
- })
- .collect()
+ fn to_bytes_entry(v: impl IntoIterator<Item = (i32, Datum)>) ->
Result<Vec<BytesEntry>, Error> {
+ let mut bs = vec![];
Review Comment:
nit: It would be better to preallocate with iterator's size
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
