spicysharkco opened a new pull request, #3476: URL: https://github.com/apache/iceberg-rust/pull/3476
## Which issue does this PR close? - Part of #3409. ## What changes are included in this PR? `RawLiteral::try_into` for `PrimitiveType::Float` rejected a double when `(f64::from(v as f32) - v).abs() > f32::EPSILON`. That is an absolute bound, so valid floats such as `100.1` and `123.456` failed with `Unable to convert raw literal (double) fail convert to type float for: type mismatch`. This path reads float partition values in `deserialize_data_file_from_json` and float `Datum` values. The check is replaced by a plain `v as f32`. That is what iceberg-java does: `SingleValueParser.fromJson` checks `isFloatingPointNumber()` for FLOAT and returns `floatValue()` ([SingleValueParser.java L73-L79](https://github.com/apache/iceberg/blob/2cf68d951533659dd18ccdfa6501d925ecd52d02/core/src/main/java/org/apache/iceberg/SingleValueParser.java#L73-L79)). It is also what `Literal::try_from_json` already does for field defaults. A double with no exact f32 now narrows to the nearest one: `16777217.0` gives `16777216.0`, where main returned an error. Doubles outside the f32 range (for example `1e39`, which becomes infinity) behave as on main and as `floatValue()`. That is the subject of #3333 and is not touched here. Notes for reviewers: - #3333 edits the line this PR removes and relies on the check to reject overflow. Its `test_raw_double_to_float_precision_loss` also expects an error for `123.456789` and `123456789.0`, which this PR accepts. If this lands first, #3333 would need its own overflow check and that test would change. - The non-finite half of #3409 (NaN and +/-Infinity written as JSON `null`) is left open. Returning an error from `Literal::try_into_json` would make `From<NestedField> for SerdeNestedField` panic at `datatypes.rs:649-650` (`.expect` on `try_into_json`) for a field whose default is NaN or infinite. Java's `SingleValueParser.toJson` writes quoted `"NaN"` and `"Infinity"` strings by default (Jackson `QUOTE_NON_NUMERIC_NUMBERS`), which its own `fromJson` cannot read back. Which behaviour do you want here? ## Are these changes tested? Unit tests in `crates/iceberg/src/spec/values/tests.rs`, which fail on current main and pass with this change: - `raw_literal_json_number_to_float`: `100.1`, `123.456`, `-123.456`, `f32::MAX`, `f32::MIN` and the smallest subnormal, parsed with `serde_json::from_str::<RawLiteral>`, convert to exactly that f32. - `raw_literal_avro_double_to_float`: the same values through `Value::Double`, both as the widened f32 and as the shortest decimal parsed to f64. - `raw_literal_double_without_exact_float_narrows_to_nearest`: `16777217.0` becomes `16777216.0`, and `1.0000000001` becomes `1.0`. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
