comphead commented on code in PR #5840:
URL: https://github.com/apache/datafusion-comet/pull/5840#discussion_r4017815957
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -136,95 +137,159 @@ macro_rules! cast_float_to_timestamp_impl {
}};
}
-macro_rules! cast_float_to_string {
- ($from:expr, $eval_mode:expr, $type:ty, $output_type:ty, $offset_type:ty,
$min_value:expr) => {{
-
- fn cast<OffsetSize>(
- from: &dyn Array,
- _eval_mode: EvalMode,
- ) -> SparkResult<ArrayRef>
- where
- OffsetSize: OffsetSizeTrait, {
- use std::fmt::Write;
-
- let array =
from.as_any().downcast_ref::<$output_type>().unwrap();
-
- // If the absolute number is less than 10,000,000 and greater
or equal than 0.001, the
- // result is expressed without scientific notation with at
least one digit on either side of
- // the decimal point. Otherwise, Spark uses a mantissa
followed by E and an
- // exponent. The mantissa has an optional leading minus sign
followed by one digit to the
- // left of the decimal point, and the minimal number of digits
greater than zero to the
- // right. The exponent has and optional leading minus sign.
- // source:
https://docs.databricks.com/en/sql/language-manual/functions/cast.html
-
- const LOWER_SCIENTIFIC_BOUND: $type = 0.001;
- const UPPER_SCIENTIFIC_BOUND: $type = 10000000.0;
-
- // Values are formatted straight into the builder, so no
intermediate String
- // is allocated per row. Capacity hint matches arrow-rs's own
AVERAGE_STRING_LENGTH
- // (16 bytes / value) so typical fractional and scientific
outputs like
- // "1234.5678" or "-1.4E-45" do not force a mid-loop grow.
- let mut builder =
GenericStringBuilder::<OffsetSize>::with_capacity(
- array.len(),
- array.len() * 16,
- );
- // Reused across rows by the scientific-notation path, which
has to inspect
- // the formatted text before emitting it.
- let mut scratch = String::with_capacity(32);
-
- for value in array.iter() {
- let Some(value) = value else {
- builder.append_null();
- continue;
- };
- let abs = value.abs();
- if
(LOWER_SCIENTIFIC_BOUND..UPPER_SCIENTIFIC_BOUND).contains(&abs)
- || abs == 0.0
- {
- let _ = write!(builder, "{value}");
- if value.fract() == 0.0 {
- // Spark always renders a fractional digit; Rust
omits it.
- let _ = builder.write_str(".0");
- }
- builder.append_value("");
- } else if !value.is_finite() {
- // NaN and the infinities are excluded by the range
check above.
- builder.append_value(if value.is_nan() {
- "NaN"
- } else if value.is_sign_positive() {
- "Infinity"
- } else {
- "-Infinity"
- });
- } else if abs.to_bits() == 1 {
- // Java's Double.toString / Float.toString are not
shortest-roundtrip
- // and render the smallest subnormals with more digits
than Rust does.
- builder.append_value(if value.is_sign_negative() {
- concat!("-", $min_value)
- } else {
- $min_value
- });
- } else {
- scratch.clear();
- let _ = write!(scratch, "{value:E}");
- match scratch.split_once('E') {
- Some((coefficient, exponent)) if
!coefficient.contains('.') => {
- // Spark keeps the fractional digit Rust drops
from a whole
- // coefficient.
- let _ = builder.write_str(coefficient);
- let _ = builder.write_str(".0E");
- builder.append_value(exponent);
- }
- _ => builder.append_value(&scratch),
- }
- }
- }
+/// A float width that Java renders through `Float.toString` /
`Double.toString`.
+///
+/// `num::Float` supplies the arithmetic predicates; the two widths differ
only in the plain-notation
+/// window's endpoints and in the literal text of the smallest subnormal,
which Java's algorithm
+/// spells with more digits than a shortest-round-trip formatter produces.
+pub trait JavaFloatString: Float + fmt::Display + fmt::UpperExp {
+ /// `Float.MIN_VALUE` / `Double.MIN_VALUE` as Java spells it.
+ const MIN_SUBNORMAL: &'static str;
+ /// Plain notation covers `[0.001, 10^7)`; anything outside it is
scientific.
+ const PLAIN_LOWER: Self;
+ const PLAIN_UPPER: Self;
+
+ /// The value one ULP above zero, the one Java does not render shortest.
`Float::min_positive_value`
+ /// is the smallest *normal*, so this has no `num` equivalent.
+ fn is_smallest_subnormal(self) -> bool;
Review Comment:
Both impls spell this identically (`self.abs().to_bits() == 1`), so the
method only exists because `to_bits` is not on `num::Float`. An associated
*value* const removes the method and both bodies:
```rust
/// `Float.MIN_VALUE` / `Double.MIN_VALUE`.
const MIN_SUBNORMAL: Self; // f32::from_bits(1) / f64::from_bits(1)
/// ...as Java spells it.
const MIN_SUBNORMAL_TEXT: &'static str;
```
Call site becomes `abs == T::MIN_SUBNORMAL`. `from_bits` is `const` well
below the 1.94 MSRV. Renaming the string const also stops `MIN_SUBNORMAL`
reading like the value rather than its spelling.
##########
native/spark-expr/src/conversion_funcs/mod.rs:
##########
@@ -23,4 +23,5 @@ mod temporal;
pub(crate) mod trim;
mod utils;
+pub use numeric::{write_java_float_string, JavaFloatString};
Review Comment:
`lib.rs` re-exports with `pub use conversion_funcs::*`, so this makes
`JavaFloatString` part of the published `datafusion-comet-spark-expr` API and
lets a downstream crate implement it for its own type. The only external
consumer needs `f32`/`f64` -> `String`.
Either seal the trait (private supertrait) or export just the owned helper,
e.g. `pub fn java_float_string<T: JavaFloatString>(v: T) -> String`. The second
also lets `iceberg_partition_path.rs` drop its local wrapper and the
`JavaFloatString` import.
##########
native/core/src/execution/operators/iceberg_partition_path.rs:
##########
@@ -170,7 +170,8 @@ fn human_string(transform: &Transform, field_type: &Type,
value: Option<&Literal
// `year`/`month`/`day`/`hour` render the ordinal itself and never see a
timestamp or binary
// field type (their result types are `int` and `date`), so they cannot
collide with the arms
- // below. iceberg-rust already mirrors `TransformUtil` for them.
+ // below. iceberg-rust already mirrors `TransformUtil` for them. `bucket`
and `truncate` reject
Review Comment:
The guarantee that holds here is stronger than the one stated, and does not
depend on what `bucket` accepts: `partition_to_path` passes
`partition_type.fields()[index].field_type`, i.e. the transform's *result*
type, so `bucket` presents `int` and can never reach a `Float`/`Double` arm
even for a float source column (iceberg-java did allow bucketing float/double
before deprecating it in 1.3). `truncate` keeps the source type but has no
float/double arm, so `identity` is indeed the only way in.
Suggest resting the comment on the result-type argument instead.
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteActionSuite.scala:
##########
@@ -1110,6 +1110,42 @@ class CometIcebergWriteActionSuite
}
}
+ // iceberg-java renders a `float` or `double` partition value with
`Float.toString` /
+ // `Double.toString`. Rust's `Display` spelled `Double.MAX_VALUE` as 309
digits instead, past the
+ // 255-byte limit on one path component (apache/datafusion-comet#5836).
+ test("native acceleration: float and double partition paths match
iceberg-java") {
Review Comment:
Third instance of this shape in the suite: `ts_path_native`/`ts_path_jvm`
and `escaped_native`/`escaped_jvm` also create a table pair, insert identical
`VALUES`, and compare `partitionDirs`. A helper taking (base name, column DDL,
partition spec, `VALUES`, expected dirs) would collapse all three.
Two other things:
1. Is `PARTITIONED BY (f, d)` accepted on every Iceberg version the suite
runs against? Float/double partitioning has been deprecated since 1.3, and the
neighbouring path-spelling test gates its JVM comparison on
`icebergVersionAtLeast(1, 8)`. If an older or newer profile rejects the `CREATE
TABLE`, this fails for an unrelated reason.
2. Unlike the other two path tests, this one does not read the rows back
through both readers -- `assertNativeWriteEngages` only checks `id`. Since the
bug was that the directory could not be created at all, a `Seq("true",
"false").foreach { cometEnabled => ... }` readback of `f`/`d` would confirm a
`d=4.9E-324` directory is openable by both.
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -136,95 +137,159 @@ macro_rules! cast_float_to_timestamp_impl {
}};
}
-macro_rules! cast_float_to_string {
- ($from:expr, $eval_mode:expr, $type:ty, $output_type:ty, $offset_type:ty,
$min_value:expr) => {{
-
- fn cast<OffsetSize>(
- from: &dyn Array,
- _eval_mode: EvalMode,
- ) -> SparkResult<ArrayRef>
- where
- OffsetSize: OffsetSizeTrait, {
- use std::fmt::Write;
-
- let array =
from.as_any().downcast_ref::<$output_type>().unwrap();
-
- // If the absolute number is less than 10,000,000 and greater
or equal than 0.001, the
- // result is expressed without scientific notation with at
least one digit on either side of
- // the decimal point. Otherwise, Spark uses a mantissa
followed by E and an
- // exponent. The mantissa has an optional leading minus sign
followed by one digit to the
- // left of the decimal point, and the minimal number of digits
greater than zero to the
- // right. The exponent has and optional leading minus sign.
- // source:
https://docs.databricks.com/en/sql/language-manual/functions/cast.html
-
- const LOWER_SCIENTIFIC_BOUND: $type = 0.001;
- const UPPER_SCIENTIFIC_BOUND: $type = 10000000.0;
-
- // Values are formatted straight into the builder, so no
intermediate String
- // is allocated per row. Capacity hint matches arrow-rs's own
AVERAGE_STRING_LENGTH
- // (16 bytes / value) so typical fractional and scientific
outputs like
- // "1234.5678" or "-1.4E-45" do not force a mid-loop grow.
- let mut builder =
GenericStringBuilder::<OffsetSize>::with_capacity(
- array.len(),
- array.len() * 16,
- );
- // Reused across rows by the scientific-notation path, which
has to inspect
- // the formatted text before emitting it.
- let mut scratch = String::with_capacity(32);
-
- for value in array.iter() {
- let Some(value) = value else {
- builder.append_null();
- continue;
- };
- let abs = value.abs();
- if
(LOWER_SCIENTIFIC_BOUND..UPPER_SCIENTIFIC_BOUND).contains(&abs)
- || abs == 0.0
- {
- let _ = write!(builder, "{value}");
- if value.fract() == 0.0 {
- // Spark always renders a fractional digit; Rust
omits it.
- let _ = builder.write_str(".0");
- }
- builder.append_value("");
- } else if !value.is_finite() {
- // NaN and the infinities are excluded by the range
check above.
- builder.append_value(if value.is_nan() {
- "NaN"
- } else if value.is_sign_positive() {
- "Infinity"
- } else {
- "-Infinity"
- });
- } else if abs.to_bits() == 1 {
- // Java's Double.toString / Float.toString are not
shortest-roundtrip
- // and render the smallest subnormals with more digits
than Rust does.
- builder.append_value(if value.is_sign_negative() {
- concat!("-", $min_value)
- } else {
- $min_value
- });
- } else {
- scratch.clear();
- let _ = write!(scratch, "{value:E}");
- match scratch.split_once('E') {
- Some((coefficient, exponent)) if
!coefficient.contains('.') => {
- // Spark keeps the fractional digit Rust drops
from a whole
- // coefficient.
- let _ = builder.write_str(coefficient);
- let _ = builder.write_str(".0E");
- builder.append_value(exponent);
- }
- _ => builder.append_value(&scratch),
- }
- }
- }
+/// A float width that Java renders through `Float.toString` /
`Double.toString`.
+///
+/// `num::Float` supplies the arithmetic predicates; the two widths differ
only in the plain-notation
+/// window's endpoints and in the literal text of the smallest subnormal,
which Java's algorithm
+/// spells with more digits than a shortest-round-trip formatter produces.
+pub trait JavaFloatString: Float + fmt::Display + fmt::UpperExp {
+ /// `Float.MIN_VALUE` / `Double.MIN_VALUE` as Java spells it.
+ const MIN_SUBNORMAL: &'static str;
+ /// Plain notation covers `[0.001, 10^7)`; anything outside it is
scientific.
+ const PLAIN_LOWER: Self;
+ const PLAIN_UPPER: Self;
+
+ /// The value one ULP above zero, the one Java does not render shortest.
`Float::min_positive_value`
+ /// is the smallest *normal*, so this has no `num` equivalent.
+ fn is_smallest_subnormal(self) -> bool;
+}
- Ok(Arc::new(builder.finish()))
+impl JavaFloatString for f32 {
+ const MIN_SUBNORMAL: &'static str = "1.4E-45";
+ const PLAIN_LOWER: Self = 0.001;
+ const PLAIN_UPPER: Self = 10000000.0;
+
+ fn is_smallest_subnormal(self) -> bool {
+ self.abs().to_bits() == 1
+ }
+}
+
+impl JavaFloatString for f64 {
+ const MIN_SUBNORMAL: &'static str = "4.9E-324";
+ const PLAIN_LOWER: Self = 0.001;
+ const PLAIN_UPPER: Self = 10000000.0;
+
+ fn is_smallest_subnormal(self) -> bool {
+ self.abs().to_bits() == 1
+ }
+}
+
+/// Writes `value` as Java's `Float.toString` / `Double.toString` renders it.
+///
+/// If the absolute value is less than 10,000,000 and greater or equal than
0.001, the result is
+/// expressed without scientific notation with at least one digit on either
side of the decimal
+/// point. Otherwise the value is a mantissa followed by `E` and an exponent,
the mantissa having
+/// an optional leading minus sign followed by one digit to the left of the
decimal point and the
+/// minimal number of digits greater than zero to the right.
+/// Source:
<https://docs.databricks.com/en/sql/language-manual/functions/cast.html>
+///
+/// Rust's own `Display` and `UpperExp` give the same digits but drop a whole
coefficient's
+/// fractional zero (`1` for `1.0`) and never switch to an exponent, so
`Double.MAX_VALUE` would
+/// render as 309 digits. Both matter beyond cosmetics: Spark spells a
`cast(double as string)`
+/// this way, and iceberg-java spells a float or double partition directory
this way, where the
+/// unabbreviated form overruns the filesystem's limit on one path component.
+///
+/// This is the pre-JDK-19 `Double.toString`, which is not shortest-round-trip
for every value.
+/// Only the smallest subnormal, by far the most visible case, is corrected
for here.
+///
+/// Errors only if `out` does; writing into a `String` or an arrow string
builder cannot fail.
+pub fn write_java_float_string<T: JavaFloatString, W: fmt::Write>(
+ value: T,
+ out: &mut W,
+) -> fmt::Result {
+ let abs = value.abs();
+ if (T::PLAIN_LOWER..T::PLAIN_UPPER).contains(&abs) || abs.is_zero() {
+ write!(out, "{value}")?;
+ if value.fract().is_zero() {
+ // Java always renders a fractional digit; Rust omits it.
+ out.write_str(".0")?;
+ }
+ Ok(())
+ } else if !value.is_finite() {
+ // NaN and the infinities are excluded by the range check above.
+ out.write_str(if value.is_nan() {
+ "NaN"
+ } else if value.is_sign_negative() {
+ "-Infinity"
+ } else {
+ "Infinity"
+ })
+ } else if value.is_smallest_subnormal() {
+ if value.is_sign_negative() {
+ out.write_str("-")?;
+ }
+ out.write_str(T::MIN_SUBNORMAL)
+ } else {
+ // The coefficient has to be inspected before any of it is emitted, so
it is formatted
+ // into a stack buffer rather than into `out`, which may not be
rewindable.
+ let mut scratch = ExponentBuf::default();
+ write!(scratch, "{value:E}")?;
+ let text = scratch.as_str();
+ match text.split_once('E') {
+ Some((coefficient, exponent)) if !coefficient.contains('.') => {
+ // Java keeps the fractional digit Rust drops from a whole
coefficient.
+ out.write_str(coefficient)?;
+ out.write_str(".0E")?;
+ out.write_str(exponent)
}
+ _ => out.write_str(text),
+ }
+ }
+}
- cast::<$offset_type>($from, $eval_mode)
- }};
+/// Scratch space for one `{:E}` rendering, sized past the longest a float can
produce
Review Comment:
Minor: the size is justified with an example (`-2.2250738585072014E-308`, 23
bytes) rather than with the bound. Rust's `{:E}` emits at most 17 significant
digits for an `f64`, so sign + digit + point + 16 digits + `E` + sign + 3
digits = 24 bytes worst case. Stating that makes 32 obviously safe and makes
the `Err` arm in `write_str` obviously unreachable rather than defensively so.
##########
native/core/src/execution/operators/iceberg_partition_path.rs:
##########
@@ -388,6 +402,60 @@ mod tests {
assert_eq!(civil_from_days(-719_529), (-1, 12, 31));
}
+ fn double(value: f64) -> String {
+ human_string(
+ &Transform::Identity,
+ &Type::Primitive(PrimitiveType::Double),
+ Some(&Literal::Primitive(PrimitiveLiteral::Double(value.into()))),
+ )
+ }
+
+ fn float(value: f32) -> String {
+ human_string(
+ &Transform::Identity,
+ &Type::Primitive(PrimitiveType::Float),
+ Some(&Literal::Primitive(PrimitiveLiteral::Float(value.into()))),
+ )
+ }
+
+ // Expectations taken from `Double.toString` / `Float.toString` output on
the JDK
+ // (apache/datafusion-comet#5836).
+ #[test]
+ fn renders_doubles_like_java_double_to_string() {
Review Comment:
These 27 assertions pin `write_java_float_string`, which lives in
`spark-expr`, from `core` -- and most of them already have Spark-verified
coverage:
- `numeric.rs::test_spark_cast_float_min_value_to_string` pins `1.4E-45` and
`4.9E-324` for both signs.
- `cast_array_to_string.sql:30-31,60-61` pins `3.4028235E38`, `1.4E-45`,
`1.7976931348623157E308`, `4.9E-324`, `NaN`, `+/-Infinity`.
- `cast_double_to_string.sql:23-36` pins `-0.0`, `0.0`, `+/-1.5`, `NaN`,
`+/-Infinity`, `1.0E20`, `0.001`.
What is genuinely new is the plain-notation window (`9.99E-4`, `9999999.0`,
`1.0E7`), `f64::MAX`, `f64::MIN_POSITIVE`, and float coverage in general. Those
would be better as rows in `cast_double_to_string.sql` plus a new
`cast_float_to_string.sql`: `checkSparkAnswerAndOperator` compares against the
Spark running in CI rather than against strings transcribed from a JDK, which
matters here because the renderer deliberately tracks JDK 19+
shortest-round-trip output while the doc calls it pre-JDK-19.
Then two smoke assertions here are enough to prove the two new match arms
are wired.
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -136,95 +137,159 @@ macro_rules! cast_float_to_timestamp_impl {
}};
}
-macro_rules! cast_float_to_string {
- ($from:expr, $eval_mode:expr, $type:ty, $output_type:ty, $offset_type:ty,
$min_value:expr) => {{
-
- fn cast<OffsetSize>(
- from: &dyn Array,
- _eval_mode: EvalMode,
- ) -> SparkResult<ArrayRef>
- where
- OffsetSize: OffsetSizeTrait, {
- use std::fmt::Write;
-
- let array =
from.as_any().downcast_ref::<$output_type>().unwrap();
-
- // If the absolute number is less than 10,000,000 and greater
or equal than 0.001, the
- // result is expressed without scientific notation with at
least one digit on either side of
- // the decimal point. Otherwise, Spark uses a mantissa
followed by E and an
- // exponent. The mantissa has an optional leading minus sign
followed by one digit to the
- // left of the decimal point, and the minimal number of digits
greater than zero to the
- // right. The exponent has and optional leading minus sign.
- // source:
https://docs.databricks.com/en/sql/language-manual/functions/cast.html
-
- const LOWER_SCIENTIFIC_BOUND: $type = 0.001;
- const UPPER_SCIENTIFIC_BOUND: $type = 10000000.0;
-
- // Values are formatted straight into the builder, so no
intermediate String
- // is allocated per row. Capacity hint matches arrow-rs's own
AVERAGE_STRING_LENGTH
- // (16 bytes / value) so typical fractional and scientific
outputs like
- // "1234.5678" or "-1.4E-45" do not force a mid-loop grow.
- let mut builder =
GenericStringBuilder::<OffsetSize>::with_capacity(
- array.len(),
- array.len() * 16,
- );
- // Reused across rows by the scientific-notation path, which
has to inspect
- // the formatted text before emitting it.
- let mut scratch = String::with_capacity(32);
-
- for value in array.iter() {
- let Some(value) = value else {
- builder.append_null();
- continue;
- };
- let abs = value.abs();
- if
(LOWER_SCIENTIFIC_BOUND..UPPER_SCIENTIFIC_BOUND).contains(&abs)
- || abs == 0.0
- {
- let _ = write!(builder, "{value}");
- if value.fract() == 0.0 {
- // Spark always renders a fractional digit; Rust
omits it.
- let _ = builder.write_str(".0");
- }
- builder.append_value("");
- } else if !value.is_finite() {
- // NaN and the infinities are excluded by the range
check above.
- builder.append_value(if value.is_nan() {
- "NaN"
- } else if value.is_sign_positive() {
- "Infinity"
- } else {
- "-Infinity"
- });
- } else if abs.to_bits() == 1 {
- // Java's Double.toString / Float.toString are not
shortest-roundtrip
- // and render the smallest subnormals with more digits
than Rust does.
- builder.append_value(if value.is_sign_negative() {
- concat!("-", $min_value)
- } else {
- $min_value
- });
- } else {
- scratch.clear();
- let _ = write!(scratch, "{value:E}");
- match scratch.split_once('E') {
- Some((coefficient, exponent)) if
!coefficient.contains('.') => {
- // Spark keeps the fractional digit Rust drops
from a whole
- // coefficient.
- let _ = builder.write_str(coefficient);
- let _ = builder.write_str(".0E");
- builder.append_value(exponent);
- }
- _ => builder.append_value(&scratch),
- }
- }
- }
+/// A float width that Java renders through `Float.toString` /
`Double.toString`.
+///
+/// `num::Float` supplies the arithmetic predicates; the two widths differ
only in the plain-notation
+/// window's endpoints and in the literal text of the smallest subnormal,
which Java's algorithm
+/// spells with more digits than a shortest-round-trip formatter produces.
+pub trait JavaFloatString: Float + fmt::Display + fmt::UpperExp {
+ /// `Float.MIN_VALUE` / `Double.MIN_VALUE` as Java spells it.
+ const MIN_SUBNORMAL: &'static str;
+ /// Plain notation covers `[0.001, 10^7)`; anything outside it is
scientific.
+ const PLAIN_LOWER: Self;
+ const PLAIN_UPPER: Self;
+
+ /// The value one ULP above zero, the one Java does not render shortest.
`Float::min_positive_value`
+ /// is the smallest *normal*, so this has no `num` equivalent.
+ fn is_smallest_subnormal(self) -> bool;
+}
- Ok(Arc::new(builder.finish()))
+impl JavaFloatString for f32 {
+ const MIN_SUBNORMAL: &'static str = "1.4E-45";
+ const PLAIN_LOWER: Self = 0.001;
+ const PLAIN_UPPER: Self = 10000000.0;
+
+ fn is_smallest_subnormal(self) -> bool {
+ self.abs().to_bits() == 1
+ }
+}
+
+impl JavaFloatString for f64 {
+ const MIN_SUBNORMAL: &'static str = "4.9E-324";
+ const PLAIN_LOWER: Self = 0.001;
+ const PLAIN_UPPER: Self = 10000000.0;
+
+ fn is_smallest_subnormal(self) -> bool {
+ self.abs().to_bits() == 1
+ }
+}
+
+/// Writes `value` as Java's `Float.toString` / `Double.toString` renders it.
+///
+/// If the absolute value is less than 10,000,000 and greater or equal than
0.001, the result is
+/// expressed without scientific notation with at least one digit on either
side of the decimal
+/// point. Otherwise the value is a mantissa followed by `E` and an exponent,
the mantissa having
+/// an optional leading minus sign followed by one digit to the left of the
decimal point and the
+/// minimal number of digits greater than zero to the right.
+/// Source:
<https://docs.databricks.com/en/sql/language-manual/functions/cast.html>
+///
+/// Rust's own `Display` and `UpperExp` give the same digits but drop a whole
coefficient's
+/// fractional zero (`1` for `1.0`) and never switch to an exponent, so
`Double.MAX_VALUE` would
+/// render as 309 digits. Both matter beyond cosmetics: Spark spells a
`cast(double as string)`
+/// this way, and iceberg-java spells a float or double partition directory
this way, where the
+/// unabbreviated form overruns the filesystem's limit on one path component.
+///
+/// This is the pre-JDK-19 `Double.toString`, which is not shortest-round-trip
for every value.
+/// Only the smallest subnormal, by far the most visible case, is corrected
for here.
+///
+/// Errors only if `out` does; writing into a `String` or an arrow string
builder cannot fail.
Review Comment:
The same fact -- writing into a `String` or a string builder cannot fail --
is stated three times: here, at `spark_cast_float_to_utf8`'s `let _ =`, and in
`java_float_string` in `iceberg_partition_path.rs`. One statement on the
function that returns the `fmt::Result` is enough; the two `let _ =` sites can
just point at it.
--
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]
