andygrove commented on issue #4959:
URL:
https://github.com/apache/datafusion-comet/issues/4959#issuecomment-5133903339
I dug into the "Scope" note above and the divergence turns out to be
considerably wider than boolean, so I've opened #5149 as an epic and I'd
suggest closing this one in favour of it.
Short version of what the audit found:
- **7 of the 8 string cast targets diverge**, not just boolean. Only
`CAST(string AS date)` is correct, because `date_parser`'s local
`is_whitespace_or_iso_control` happens to compute exactly Spark's `trimAll` set.
- **There is a second, unreported direction.** `str::trim` also trims
non-ASCII Unicode whitespace (`U+0085`, `U+00A0`, `U+1680`, `U+2000`-`U+200A`,
`U+2028`, `U+2029`, `U+202F`, `U+205F`, `U+3000`), which Spark never trims for
any cast target. So `CAST('　true' AS boolean)` returns `true` in Comet and
`NULL` in Spark, and under ANSI Comet silently succeeds where Spark throws.
That's a silent-wrong-result path, arguably worse than the
NULL-instead-of-value direction described here.
- **Spark has two trim regimes, not one.** boolean / byte / short / int /
long / date / timestamp / timestamp_ntz use the `trimAll` set (bytes
`0x00`-`0x20` **and `0x7F`**), but float / double / decimal go through Java
`String.trim`, which is chars `U+0000`-`U+0020` and **does not** include
`0x7F`. So a single shared trim helper applied uniformly would fix boolean and
integrals while introducing a *new* divergence in float and decimal.
- One extra byte to watch: Rust's `u8::is_ascii_whitespace` excludes `0x0B`
(vertical tab) whereas `str::trim` includes it, so the integral and decimal
paths (which use `trim_ascii` / `is_ascii_whitespace`) fail on `0x0B` as well.
Verified empirically in both directions: Spark ground truth via a Java probe
against the 3.5.5 jars exercising the exact primitives each cast target uses
(`UTF8String.trimAll`, `UTF8String.toInt`/`toLong`, `Double.parseDouble`, `new
BigDecimal(String.trim)`, `DateTimeUtils.stringToDate`/`stringToTimestamp`),
and Comet via the same 25-codepoint matrix through `spark_cast` in legacy and
ANSI mode. The Spark code paths are identical in 3.5.8 and 4.1.1. Full matrix
is in #5149.
On the "mark cast(string to boolean) as incompatible" suggestion: I'd lean
against it as the resolution here, since it would leave the other six targets
silently wrong while singling out the one that was noticed first. All of them
are `Compatible()` in `CometCast.canCastFromString` today. #5149 breaks the
work down per target, and the two trim helpers it proposes should make each one
a small change.
--
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]
