Skip to content

cast string to boolean: trim ISO control bytes to match Spark's UTF8String.trimAll #4959

Closed

Description

@andygrove

Describe the bug

Comet's parse_utf8_to_boolean (native/spark-expr/src/conversion_funcs/string.rs) uses Rust str::trim, which strips only Unicode char::is_whitespace code points. Spark's UTF8String.trimAll() (referenced by Cast for stringToBool) trims every byte where Character.isWhitespace(b) || Character.isISOControl(b) (UTF8String.java:201-203, UTF8String.java:994-1010).

The two sets differ on the ASCII control bytes 0x00-0x08, 0x0E-0x1F, and 0x7F: Spark trims them, Comet does not.

Repro (per @mbutrovich on #4914)

`\" \x01true\x01 \"` -> Spark casts to `true`, Comet casts to `null` (or ANSI-errors).

Scope

Same divergence likely exists in other trim-based cast paths in conversion_funcs/string.rs. UTF8String.trimAll parity should be assessed systematically.

Prior context

Reported in the review of #4914; that PR intentionally left the pre-existing behavior alone to keep it a pure perf change.

Activity

  1. added
    priority:criticalData corruption, silent wrong results, security issues
    and removed on Jul 20, 2026
  2. kazuyukitanimura commented on Jul 20, 2026

    @kazuyukitanimura
    Contributor

    Let's mark cast(string to boolean) as incompatible (narrow down if possible)

  3. self-assigned this
    on Jul 20, 2026
  4. added this to the 1.0.0 milestone on Jul 27, 2026
  5. andygrove commented on Jul 30, 2026

    @andygrove
    MemberAuthor

    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.

  6. andygrove commented on Jul 30, 2026

    @andygrove
    MemberAuthor

    @kazuyukitanimura @mbutrovich I'll go ahead and close this issue because #5149 now supersedes it (see previous comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:expressionsExpression evaluationbugSomething isn't workingcorrectnesspriority:criticalData corruption, silent wrong results, security issues

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions