mbutrovich commented on code in PR #4918:
URL: https://github.com/apache/datafusion-comet/pull/4918#discussion_r3591124590
##########
native/spark-expr/src/conversion_funcs/numeric.rs:
##########
@@ -155,53 +157,65 @@ macro_rules! cast_float_to_string {
const LOWER_SCIENTIFIC_BOUND: $type = 0.001;
const UPPER_SCIENTIFIC_BOUND: $type = 10000000.0;
- let output_array = array
- .iter()
- .map(|value| match value {
- Some(value) if value == <$type>::INFINITY =>
Ok(Some("Infinity".to_string())),
- Some(value) if value == <$type>::NEG_INFINITY =>
Ok(Some("-Infinity".to_string())),
- Some(value)
- if (value.abs() < UPPER_SCIENTIFIC_BOUND
- && value.abs() >= LOWER_SCIENTIFIC_BOUND)
- || value.abs() == 0.0 =>
- {
- let trailing_zero = if value.fract() == 0.0 { ".0"
} else { "" };
-
- Ok(Some(format!("{value}{trailing_zero}")))
+ // Values are formatted straight into the builder, so no
intermediate String
+ // is allocated per row.
+ let mut builder =
GenericStringBuilder::<OffsetSize>::with_capacity(
+ array.len(),
+ array.len() * 8,
Review Comment:
`GenericStringBuilder::<OffsetSize>::with_capacity(array.len(), array.len()
* 8)`. Eight bytes
per value under-provisions typical fractional output like `1234.5678` (9
bytes) or the
scientific forms this cast produces (`4.9E-324` is 8, `-1.4E-45` is 8,
longer with more
mantissa digits), so the value buffer will still reallocate on realistic
data. arrow-rs itself
uses an `AVERAGE_STRING_LENGTH` of 16 for its own capacity helper
(arrow-array/src/builder/generic_bytes_builder.rs:376).
Suggested change: raise the hint to `array.len() * 16` to match arrow-rs's
own average and
avoid mid-loop growth on ordinary float text. The cost of a slightly large
preallocation is
negligible per that same arrow-rs doc note.
--
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]
