viirya commented on code in PR #6841:
URL: https://github.com/apache/datafusion-comet/pull/6841#discussion_r4236822159
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -461,38 +461,83 @@ fn cast_string_to_decimal256_impl(
))
}
-/// Normalize fullwidth Unicode digits (U+FF10–U+FF19) to their ASCII
equivalents.
+/// Code point of the zero of each run of ten non-ASCII decimal digits
(Unicode general
+/// category `Nd`) in the Basic Multilingual Plane, in ascending order. Every
run is contiguous,
+/// so the digit at `zero + n` has value `n`.
///
-/// Spark's UTF8String parser treats fullwidth digits as numerically
equivalent to
-/// ASCII digits, e.g. "１２３.４５" parses as 123.45. Each fullwidth digit encodes
-/// to exactly three UTF-8 bytes: [0xEF, 0xBC, 0x90+n] for digit n. The ASCII
-/// equivalent is 0x30+n, so the conversion is: third_byte - 0x60.
+/// This is the set of `char`s for which Java's `Character.isDigit(char)` is
true, minus ASCII.
+/// It is identical under JDK 17 (Unicode 13) and JDK 21 (Unicode 15); all
`Nd` digits added
+/// since lie outside the BMP.
+const BMP_NON_ASCII_DIGIT_ZEROS: [u16; 36] = [
+ 0x0660, // Arabic-Indic
+ 0x06F0, // Extended Arabic-Indic
+ 0x07C0, // NKo
+ 0x0966, // Devanagari
+ 0x09E6, // Bengali
+ 0x0A66, // Gurmukhi
+ 0x0AE6, // Gujarati
+ 0x0B66, // Oriya
+ 0x0BE6, // Tamil
+ 0x0C66, // Telugu
+ 0x0CE6, // Kannada
+ 0x0D66, // Malayalam
+ 0x0DE6, // Sinhala Lith
+ 0x0E50, // Thai
+ 0x0ED0, // Lao
+ 0x0F20, // Tibetan
+ 0x1040, // Myanmar
+ 0x1090, // Myanmar Shan
+ 0x17E0, // Khmer
+ 0x1810, // Mongolian
+ 0x1946, // Limbu
+ 0x19D0, // New Tai Lue
+ 0x1A80, // Tai Tham Hora
+ 0x1A90, // Tai Tham Tham
+ 0x1B50, // Balinese
+ 0x1BB0, // Sundanese
+ 0x1C40, // Lepcha
+ 0x1C50, // Ol Chiki
+ 0xA620, // Vai
+ 0xA8D0, // Saurashtra
+ 0xA900, // Kayah Li
+ 0xA9D0, // Javanese
+ 0xA9F0, // Myanmar Tai Laing
+ 0xAA50, // Cham
+ 0xABF0, // Meetei Mayek
+ 0xFF10, // Fullwidth
+];
+
+/// The value of `c` if it is a non-ASCII decimal digit that Java's
`BigDecimal` accepts.
///
-/// All other bytes (ASCII or other multi-byte sequences) are passed through
-/// unchanged, so the output is valid UTF-8 whenever the input is.
-fn normalize_fullwidth_digits(s: &str) -> String {
- let bytes = s.as_bytes();
- let mut out = Vec::with_capacity(s.len());
- let mut i = 0;
- while i < bytes.len() {
- if i + 2 < bytes.len()
- && bytes[i] == 0xEF
- && bytes[i + 1] == 0xBC
- && bytes[i + 2] >= 0x90
- && bytes[i + 2] <= 0x99
- {
- // e.g. 0x91 - 0x60 = 0x31 = b'1'
- out.push(bytes[i + 2] - 0x60);
- i += 3;
- } else {
- out.push(bytes[i]);
- i += 1;
- }
+/// `BigDecimal` scans UTF-16 code units and tests each with
`Character.isDigit(char)`, so only
+/// BMP digits count: a supplementary digit such as U+1D7CE MATHEMATICAL BOLD
DIGIT ZERO is a
+/// surrogate pair, neither half of which is a digit, and the parse fails.
+#[inline]
+fn non_ascii_digit_value(c: char) -> Option<u8> {
+ let cp = u32::from(c);
+ if cp > 0xFFFF {
+ return None;
}
- // SAFETY: we only replace valid 3-byte UTF-8 sequences [EF BC 9X] with a
- // single ASCII byte; all other bytes are copied unchanged, preserving the
- // UTF-8 invariant of the input.
- unsafe { String::from_utf8_unchecked(out) }
+ let idx = BMP_NON_ASCII_DIGIT_ZEROS.partition_point(|&zero|
u32::from(zero) <= cp);
+ let zero = u32::from(BMP_NON_ASCII_DIGIT_ZEROS[idx.checked_sub(1)?]);
+ let value = cp - zero;
+ (value < 10).then_some(value as u8)
+}
+
+/// Replace every non-ASCII decimal digit that Java's `BigDecimal` accepts
with its ASCII
+/// equivalent, e.g. "١٢٣.٤٥" and "１２３.４５" both become "123.45".
+///
+/// Spark parses decimals with `new java.math.BigDecimal(str)`, which reads
each digit through
+/// `Character.digit(c, 10)` in both the mantissa and the exponent, so digits
from any script
+/// can be mixed freely ("1٢e३" is 12000). All other characters are kept
unchanged and are
+/// rejected later by [`parse_decimal_str`].
+fn normalize_unicode_digits(s: &str) -> String {
+ s.chars()
+ .map(|c| match non_ascii_digit_value(c) {
Review Comment:
Thanks, reproduced with the `cast_from_string` benchmark (8,192 rows,
Legacy, `DECIMAL(38,10)`): fullwidth went from ~393 µs to ~822 µs per batch and
mixed ASCII/fullwidth from ~391 µs to ~973 µs, with pure ASCII unchanged.
Fixed in 04c9f7a95: the normalizer now scans bytes, copies ASCII directly,
maps the fullwidth encoding `[0xEF, 0xBC, 0x90 + n]` in place as before, and
only decodes a char for the table lookup otherwise, writing into a buffer
preallocated to the input length. I also added fullwidth, mixed ASCII/fullwidth
and Arabic-Indic cases to `cast_from_string`.
Alternating main and this commit, two rounds each:
| input | main | this PR (before) | this PR (now) |
|---|---|---|---|
| ASCII | 136–138 µs | 132 µs | 133–134 µs |
| fullwidth | 387–391 µs | 822 µs | 387–408 µs |
| mixed ASCII/fullwidth | 381–391 µs | 973 µs | 376–391 µs |
| Arabic-Indic | 1.04–1.05 ms (NULL) | 800 µs | 535–566 µs |
--
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]
