ZENOTME commented on code in PR #406:
URL: https://github.com/apache/iceberg-rust/pull/406#discussion_r1642947050
##########
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:
> Surely in almost all cases precision will be lost unless the last 29 bits
of the f64's fraction are all zeroes (which is about 1 in half a billion), as
this comparison will be bitwise?
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.
I'm not sure wether my understand of float point is correct, so please
correct me if I'm wrong.
--
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
