sdd commented on code in PR #406:
URL: https://github.com/apache/iceberg-rust/pull/406#discussion_r1644888305
##########
crates/iceberg/src/spec/values.rs:
##########
@@ -2334,9 +2453,20 @@ mod _serde {
},
RawLiteralEnum::Float(v) => match ty {
Type::Primitive(PrimitiveType::Float) =>
Ok(Some(Literal::float(v))),
+ Type::Primitive(PrimitiveType::Double) => {
+ Ok(Some(Literal::double(f64::from(v))))
+ }
_ => Err(invalid_err("float")),
},
RawLiteralEnum::Double(v) => match ty {
+ Type::Primitive(PrimitiveType::Float) => {
+ let v = v as f32;
+ // Return error if the value has lost precision
+ if v as f64 != f64::from(v) {
Review Comment:
> I think the check here is to deal with the case that serialize f32 to f64
and deserialize back to f32. In this case, there is no precision loss.
For the case loss precision, which means that the deserializer wants to
deserialize a f64 to f32. I think this means something wrong happened. 🤔
I think here, what we need to check is whether this f64 is compatible with
f32, or says, whether this f64 can be converted to f32 without losing
precision. This means that this f64 can convert from a f32.
Thanks for the clarification - that makes total sense 😄
> So I think we need to do the bitwise comparison here rather than compare
two f64. We can convert them to u64 to achieve this.
Not quite, as there are many different bit patterns that correspond to NaN,
so I think that one needs to be handled separately.
> Oh I see. How about the rule like this:
> let v = v as f32;
>
> if v == NaN || v == -Inf || v == Inf || v == -0.0 || v == 0.0:
> return v;
> else:
> let nv:f64 = v.into();
> if |nv - v| > f32::EPSILON:
> // there is precisison loss
> return err;
Works for me (with the caveat that you'll need to use `is_nan` rather than `
== NAN` for it to work)
--
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
