sunchao commented on code in PR #6595:
URL: https://github.com/apache/datafusion-comet/pull/6595#discussion_r4193614955
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -254,7 +255,44 @@ where
} else {
s
};
- // Rust's parse logic already handles scientific notations so we just rely
on it
+ let unsigned = pruned_float_str
+ .strip_prefix(['+', '-'])
+ .unwrap_or(pruned_float_str);
+ if unsigned.starts_with("0x") || unsigned.starts_with("0X") {
+ // lexical 1.0.6's fast path cannot mix radix 16 with a binary
exponent. Expand
+ // only the significand, preserving exact bits and rounding at the
target width.
+ // Remove after https://github.com/Alexhuszagh/rust-lexical/issues/87
is fixed.
+ let (mantissa, exponent) = unsigned[2..].split_once(['p', 'P'])?;
+ let mut binary = String::new();
+ if pruned_float_str.starts_with('-') {
+ binary.push('-');
+ }
+ for digit in mantissa.bytes() {
+ if digit == b'.' {
+ binary.push('.');
+ } else {
+ let digit = char::from(digit).to_digit(16)?;
+ for bit in (0..4).rev() {
+ binary.push(char::from(b'0' + ((digit >> bit) & 1) as u8));
+ }
+ }
+ }
+ binary.push('p');
+ binary.push_str(exponent);
+ const BINARY: u128 = NumberFormatBuilder::new()
+ .mantissa_radix(2)
+ .exponent_base(std::num::NonZeroU8::new(2))
+ .exponent_radix(std::num::NonZeroU8::new(10))
+ .required_exponent_notation(true)
+ .no_special(true)
+ .build_strict();
+ return F::from_lexical_with_options::<BINARY>(
Review Comment:
[P2] Preserve zero significands when hexadecimal exponents are large. With
`spark.sql.variant.pushVariantIntoScan=false`, store `parse_json('"0x0p1100"')`
in a Parquet Variant column `v`, then select `try_variant_get(v, '$',
'double')`. Spark returns `0.0`, but the new parser returns infinity. Strict
extraction also accepts that incorrect value, and `-0x0p1100` becomes negative
infinity instead of negative zero. These previously correct fallback queries
now silently change results. Handle validated zero significands before exponent
scaling, preserving the sign, and add FLOAT/DOUBLE regressions.
Evidence: A release-mode reproduction using the exact head’s unchanged
`parse_string_to_float` and pinned `lexical-parse-float` 1.0.6 returned DOUBLE
bits `7ff0000000000000` for `0x0p1100` and `fff0000000000000` for `-0x0p1100`.
Direct `VariantGet.variantGet` calls on Spark 4.1.3 and 4.2.0 returned bits
`0000000000000000` and `8000000000000000` in both strict and TRY modes. An
exact-head native `VariantGet::evaluate_array` probe with `0x0p157` targeting
FLOAT additionally panicked at `lexical-parse-float/src/binary.rs:40` in the
debug build. The release parser returned FLOAT infinity for that input.
--
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]
