You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/01 11:26:32 UTC
(datafusion-comet) 01/01: fix: match Spark's whitespace trimming in casts from string to timestamp and timestamp_ntz (#6445)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6445-56c5a65994cf3e3b878d347583a89e16cf7ae32e
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
commit b994a39bfa9de85e32b28cad9f6b298cc8024360
Author: Andy Grove <ag...@apache.org>
AuthorDate: Thu Oct 1 11:26:09 2026 +0000
fix: match Spark's whitespace trimming in casts from string to timestamp and timestamp_ntz (#6445)
Trim the `UTF8String.trimAll` byte set, like the other string cast targets,
instead of Unicode whitespace, and raise CAST_INVALID_INPUT under ANSI for a
value that trims to nothing.
---
.../expressions/_category_template/cast.md | 15 ++--
.../user-guide/latest/compatibility/index.md | 5 --
native/spark-expr/src/conversion_funcs/string.rs | 95 +++++++++++-----------
native/spark-expr/src/conversion_funcs/trim.rs | 12 +--
.../expressions/cast/cast_string_trim.sql | 12 ++-
.../org/apache/comet/CometNativeCastSuite.scala | 30 +++++++
6 files changed, 97 insertions(+), 72 deletions(-)
diff --git a/docs/source/user-guide/latest/compatibility/expressions/_category_template/cast.md b/docs/source/user-guide/latest/compatibility/expressions/_category_template/cast.md
index e2ace6c121..c023539f36 100644
--- a/docs/source/user-guide/latest/compatibility/expressions/_category_template/cast.md
+++ b/docs/source/user-guide/latest/compatibility/expressions/_category_template/cast.md
@@ -42,22 +42,19 @@ level as in legacy mode. See the [ANSI Mode](#ansi-mode) table below for per-pai
## Whitespace Trimming in Casts from String
-Spark uses two different notions of whitespace when casting from string. Comet reproduces both,
-with one known exception noted below:
+Spark uses two different notions of whitespace when casting from string, and Comet reproduces
+both:
-| Trim set | Cast targets |
-| -------------------------------------- | ------------------------------------------------------------------------- |
-| Bytes `0x00`-`0x20` and `0x7F` (`DEL`) | `BOOLEAN`, `TINYINT`, `SMALLINT`, `INT`, `BIGINT`, `DATE`, `TIMESTAMP` \* |
-| Bytes `0x00`-`0x20` only | `FLOAT`, `DOUBLE`, `DECIMAL` |
+| Trim set | Cast targets |
+| -------------------------------------- | --------------------------------------------------------------------------------------- |
+| Bytes `0x00`-`0x20` and `0x7F` (`DEL`) | `BOOLEAN`, `TINYINT`, `SMALLINT`, `INT`, `BIGINT`, `DATE`, `TIMESTAMP`, `TIMESTAMP_NTZ` |
+| Bytes `0x00`-`0x20` only | `FLOAT`, `DOUBLE`, `DECIMAL` |
Neither set includes any non-ASCII whitespace, so padding a value with `U+0085`, `U+00A0`,
`U+1680`, `U+2000`-`U+200A`, `U+2028`, `U+2029`, `U+202F`, `U+205F` or `U+3000` produces `NULL`
(or raises under ANSI mode), even though those codepoints are whitespace to Unicode. Whitespace
inside a value is never trimmed.
-\* `CAST(string AS TIMESTAMP)` and `CAST(string AS TIMESTAMP_NTZ)` still trim Unicode whitespace
-in Comet; see [#5149](https://github.com/apache/datafusion-comet/issues/5149).
-
## String to Decimal
Comet's native `CAST(string AS DECIMAL)` implementation matches Apache Spark's behavior,
diff --git a/docs/source/user-guide/latest/compatibility/index.md b/docs/source/user-guide/latest/compatibility/index.md
index 29c57c766a..9bae431207 100644
--- a/docs/source/user-guide/latest/compatibility/index.md
+++ b/docs/source/user-guide/latest/compatibility/index.md
@@ -127,11 +127,6 @@ The following native paths silently return values that differ from Spark for edg
Most also have entries in the per-category expression pages linked above; they are collected here
so users hunting an unexpected value have a single place to check:
-- `CAST(string AS timestamp)` and `CAST(string AS timestamp_ntz)` trim Unicode whitespace.
- Spark trims only the bytes `0x00`-`0x20` and `0x7F`, so a value padded with an ASCII control byte
- parses in Spark and returns `NULL` in Comet, while a value padded with non-ASCII whitespace such
- as `U+3000` returns `NULL` in Spark and parses in Comet
- ([#5149](https://github.com/apache/datafusion-comet/issues/5149)).
- Native `RANGE` window frames with an explicit `PRECEDING` / `FOLLOWING` offset diverge from
Spark when the boundary arithmetic overflows for `DATE` or `DECIMAL` `ORDER BY` columns
([#5022](https://github.com/apache/datafusion-comet/issues/5022)).
diff --git a/native/spark-expr/src/conversion_funcs/string.rs b/native/spark-expr/src/conversion_funcs/string.rs
index b466a5f4a0..a23e5dcc95 100644
--- a/native/spark-expr/src/conversion_funcs/string.rs
+++ b/native/spark-expr/src/conversion_funcs/string.rs
@@ -45,8 +45,9 @@ macro_rules! cast_utf8_to_timestamp {
if $array.is_null(i) {
cast_array.append_null()
} else {
- // trim_end only: leading spaces affect parsing (e.g. " T2" -> null, "T2" -> valid)
- match $cast_method($array.value(i).trim_end(), $eval_mode $(, $extra_arg)*) {
+ // Untrimmed: the parsers trim, and Spark 4 needs the raw start to reject
+ // padding before a time-only `T` (" T2" -> null, "T2" -> valid).
+ match $cast_method($array.value(i), $eval_mode $(, $extra_arg)*) {
Ok(Some(cast_value)) => cast_array.append_value(cast_value),
Ok(None) => cast_array.append_null(),
Err(e) => {
@@ -1415,15 +1416,19 @@ fn timestamp_parser<T: TimeZone>(
tz: &T,
is_spark4_plus: bool,
) -> SparkResult<Option<i64>> {
- let trimmed = value.trim();
- if trimmed.is_empty() {
- return Ok(None);
- }
- // Spark 4.0+ rejects leading whitespace for ALL T-prefixed time-only strings
- // (T<h>, T<h>:<m>, T<h>:<m>:<s>, T<h>:<m>:<s>.<f>), but accepts trailing whitespace.
- // Spark 3.x trims all whitespace first, so leading whitespace is accepted there.
+ // `SparkDateTimeUtils.getTrimmedStart`/`getTrimmedEnd` trim the same byte set as
+ // `UTF8String.trimAll`.
+ let (start, end) = trim_all_range(value.as_bytes());
+ let trimmed = &value[start..end];
+ // A value that trims to nothing has no segments, so Spark treats it as malformed like any
+ // other: null, or CAST_INVALID_INPUT under ANSI.
+ //
+ // Spark 4.0+ also rejects leading padding for ALL T-prefixed time-only strings
+ // (T<h>, T<h>:<m>, T<h>:<m>:<s>, T<h>:<m>:<s>.<f>), but accepts trailing padding: it only
+ // treats the `T` as a time-only marker at raw byte 0. Spark 3.x trims first, so leading
+ // padding is accepted there.
// Check the prefix, not the base patterns: a zone suffix can hide a time-only match.
- if is_spark4_plus && value.len() > value.trim_start().len() && trimmed.starts_with('T') {
+ if trimmed.is_empty() || (is_spark4_plus && start > 0 && trimmed.starts_with('T')) {
return if eval_mode == EvalMode::Ansi {
Err(SparkError::InvalidInputInCastToDatetime {
value: value.to_string(),
@@ -1761,18 +1766,14 @@ fn timestamp_ntz_parser(
allow_time_zone: bool,
_is_spark4_plus: bool,
) -> SparkResult<Option<i64>> {
- let trimmed = value.trim();
- if trimmed.is_empty() {
- return Ok(None);
- }
-
- // NTZ rejects leading whitespace for T-prefixed time-only strings on Spark 4+
- // (same logic as timestamp_parser), but time-only is rejected entirely for NTZ anyway.
+ // Trimmed as in `timestamp_parser`. NTZ rejects time-only strings outright, so it needs no
+ // Spark 4 check for padding before a `T`.
+ let value = trim_all(value);
- let value = trimmed;
-
- // Reject time-only patterns: NTZ requires a date component
- if RE_TIME_ONLY_H.is_match(value)
+ // Reject a value that trims to nothing, and time-only patterns: NTZ requires a date
+ // component
+ if value.is_empty()
+ || RE_TIME_ONLY_H.is_match(value)
|| RE_TIME_ONLY_HM.is_match(value)
|| RE_TIME_ONLY_HMS.is_match(value)
|| RE_TIME_ONLY_HMSU.is_match(value)
@@ -2315,32 +2316,15 @@ mod tests {
}
}
- /// Pins the one trim divergence this PR leaves behind, so that resolving
- /// <https://github.com/apache/datafusion-comet/issues/5149> has to update this test rather
- /// than change behaviour silently. `timestamp_parser` and `timestamp_ntz_parser` still use
- /// `str::trim`, so they accept the non-ASCII whitespace that Spark's
- /// `SparkDateTimeUtils.getTrimmedStart` / `getTrimmedEnd` leave in place, where Spark returns
- /// NULL. `CometNativeCastSuite` cannot cover this, because Spark is the oracle there and Comet does
- /// not fall back -- it silently returns a value.
+ /// Timestamps trim the `trimAll` set too, through `SparkDateTimeUtils.getTrimmedStart` /
+ /// `getTrimmedEnd`.
#[test]
- fn test_cast_string_to_timestamp_unicode_whitespace_divergence() {
- let to_types = [
+ fn test_cast_string_to_timestamp_trim_parity() {
+ for to_type in [
DataType::Timestamp(TimeUnit::Microsecond, Some("UTC".into())),
DataType::Timestamp(TimeUnit::Microsecond, None),
- ];
- for pad in ["\u{85}", "\u{a0}", "\u{2028}", "\u{3000}"] {
- for to_type in &to_types {
- let input = format!("{pad}2020-01-01 12:34:56{pad}");
- let array: ArrayRef = Arc::new(StringArray::from(vec![Some(input.as_str())]));
- let options = SparkCastOptions::new(EvalMode::Legacy, "UTC", false);
- let result = cast_array(array, to_type, &options).unwrap();
- assert!(
- !result.is_null(0),
- "cast {input:?} to {to_type}: Comet still trims {pad:?} where Spark returns \
- NULL. If this now returns NULL, the parsers have moved to the trim helpers \
- -- delete this test and extend `assert_trim_parity` to the timestamp targets."
- );
- }
+ ] {
+ assert_trim_parity(&to_type, "2020-01-01 12:34:56", trim_all);
}
}
@@ -2570,7 +2554,8 @@ mod tests {
#[test]
fn test_leading_whitespace_t_hm() {
let tz = &Tz::from_str("UTC").unwrap();
- // Spark 4.0+ rejects leading whitespace for ALL T-prefixed time-only patterns.
+ // Spark 4.0+ rejects leading whitespace for ALL T-prefixed time-only patterns, and the
+ // ISO control characters Spark trims along with it count as whitespace here too.
for ws_input in &[
" T2:30",
"\tT2:30",
@@ -2580,6 +2565,8 @@ mod tests {
"\nT2",
"\tT1:2:3 +08:00",
" T1:2:3.4 +08:00",
+ "\u{1}T2",
+ "\u{7f}T2:30",
] {
for mode in [EvalMode::Legacy, EvalMode::Try] {
assert!(
@@ -2603,7 +2590,14 @@ mod tests {
);
}
// Without leading whitespace, these must be valid on all versions.
- for ok_input in &["T2:30", "T2", "T1:2:3 +08:00", "T1:2:3.4 +08:00"] {
+ for ok_input in &[
+ "T2:30",
+ "T2",
+ "T1:2:3 +08:00",
+ "T1:2:3.4 +08:00",
+ "T2\u{1}",
+ "T2:30\u{7f}",
+ ] {
assert!(
timestamp_parser(ok_input, EvalMode::Legacy, tz, true)
.unwrap()
@@ -2611,6 +2605,15 @@ mod tests {
"'{ok_input}' should be valid"
);
}
+ // Non-ASCII whitespace is never trimmed, so it is malformed on every version.
+ for spark4 in [false, true] {
+ assert!(
+ timestamp_parser("\u{3000}T2", EvalMode::Legacy, tz, spark4)
+ .unwrap()
+ .is_none(),
+ "'\\u{{3000}}T2' should be null (spark4={spark4})"
+ );
+ }
}
#[test]
diff --git a/native/spark-expr/src/conversion_funcs/trim.rs b/native/spark-expr/src/conversion_funcs/trim.rs
index 8cd7159096..9e4e98d205 100644
--- a/native/spark-expr/src/conversion_funcs/trim.rs
+++ b/native/spark-expr/src/conversion_funcs/trim.rs
@@ -21,10 +21,10 @@
//! regimes, and neither of them matches Rust's `str::trim` (which trims Unicode whitespace) or
//! `<[u8]>::trim_ascii` (which omits `0x0B`):
//!
-//! | Regime | Trimmed bytes | Cast targets |
-//! |----------------------|--------------------------|----------------------------------------------------|
-//! | [`trim_all`] | `0x00`-`0x20` and `0x7F` | boolean, byte, short, int, long, date, timestamp \* |
-//! | [`trim_java_string`] | `0x00`-`0x20` | float, double, decimal |
+//! | Regime | Trimmed bytes | Cast targets |
+//! |----------------------|--------------------------|-----------------------------------------------------------------|
+//! | [`trim_all`] | `0x00`-`0x20` and `0x7F` | boolean, byte, short, int, long, date, timestamp, timestamp_ntz |
+//! | [`trim_java_string`] | `0x00`-`0x20` | float, double, decimal |
//!
//! Neither regime trims non-ASCII whitespace (`str::trim` does, which is why it cannot be reused
//! here). The `String.trim` regime comes from the JDK calls Spark delegates to:
@@ -33,10 +33,6 @@
//!
//! `to_time` and `try_to_time` also use [`trim_all`] after detecting an optional AM/PM suffix;
//! suffix detection itself first removes ASCII spaces only, matching Spark's `stringToTime`.
-//!
-//! \* `timestamp` and `timestamp_ntz` are listed for what Spark does; the Comet parsers for those
-//! two targets have not been migrated to these helpers and still use `str::trim`
-//! (<https://github.com/apache/datafusion-comet/issues/5149>).
/// True for the bytes trimmed by `org.apache.spark.unsafe.types.UTF8String.trimAll`, i.e. the
/// bytes `b` for which `Character.isWhitespace(b) || Character.isISOControl(b)` holds, which
diff --git a/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql b/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
index c14b893b0c..5180f80f04 100644
--- a/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
+++ b/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
@@ -18,9 +18,9 @@
-- ConfigMatrix: parquet.enable.dictionary=false,true
-- Whitespace trimming in cast(string as <fixed type>). Spark has two trim regimes and neither
--- trims non-ASCII whitespace, so DEL is trimmed for boolean/int/date but not for double/decimal,
--- and NBSP / U+3000 are trimmed for nothing. See conversion_funcs::trim in the native crate and
--- https://github.com/apache/datafusion-comet/issues/5149.
+-- trims non-ASCII whitespace, so DEL is trimmed for boolean/int/date/timestamp but not for
+-- double/decimal, and NBSP / U+3000 are trimmed for nothing. See conversion_funcs::trim in the
+-- native crate and https://github.com/apache/datafusion-comet/issues/5149.
--
-- Per-codepoint parity across all three eval modes is covered by the CometNativeCastSuite
-- "whitespace trim parity" tests; this fixture covers the same regimes over a Parquet column.
@@ -54,7 +54,9 @@ SELECT
cast(concat(pad, '1.5', pad) as float),
cast(concat(pad, '1.5', pad) as double),
cast(concat(pad, '1.5', pad) as decimal(10,2)),
- cast(concat(pad, '2020-01-01', pad) as date)
+ cast(concat(pad, '2020-01-01', pad) as date),
+ cast(concat(pad, '2020-01-01 12:34:56', pad) as timestamp),
+ cast(concat(pad, '2020-01-01 12:34:56', pad) as timestamp_ntz)
FROM cast_trim_pad
-- Padding on its own, and padding in the interior, must never parse
@@ -66,6 +68,8 @@ SELECT
cast(pad as double),
cast(pad as decimal(10,2)),
cast(pad as date),
+ cast(pad as timestamp),
+ cast(pad as timestamp_ntz),
cast(concat('1', pad, '2') as int),
cast(concat('1', pad, '.5') as double)
FROM cast_trim_pad
diff --git a/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala b/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
index 480fed1429..4db950e73c 100644
--- a/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
@@ -1068,6 +1068,29 @@ class CometNativeCastSuite extends CometTestBase with AdaptiveSparkPlanHelper {
.foreach(castTest(values, _))
}
+ test("cast StringType to DateType - whitespace trim parity") {
+ castTest(trimPaddedValues("2020-01-01").toDF("a"), DataTypes.DateType)
+ }
+
+ test("cast StringType to timestamp types - whitespace trim parity") {
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC") {
+ val values = trimPaddedValues("2020-01-01 12:34:56").toDF("a")
+ Seq(DataTypes.TimestampType, DataTypes.TimestampNTZType).foreach(castTest(values, _))
+ }
+ }
+
+ test("cast StringType to timestamp types - ANSI rejects a value that trims to nothing") {
+ // Spark's parseTimestampString finds no segments in such a value, so ANSI mode raises
+ // CAST_INVALID_INPUT for it like for any other malformed value. The batch-wide ANSI check in
+ // the parity test above cannot see this, since other rows in its batch raise as well.
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC") {
+ for {
+ value <- Seq("", " ", "\t", "\u0001", "\u007f")
+ toType <- Seq(DataTypes.TimestampType, DataTypes.TimestampNTZType)
+ } castTest(Seq(value).toDF("a"), toType, expectAnsiFailure = true)
+ }
+ }
+
private val castStringToIntegralInputs: Seq[String] = Seq(
"",
".",
@@ -1524,6 +1547,13 @@ class CometNativeCastSuite extends CometTestBase with AdaptiveSparkPlanHelper {
" T2:30",
"\tT2:30",
"\nT2:30",
+ // ISO control characters are trimmed like whitespace, on either side
+ "\u0001T2",
+ "\u007fT2:30",
+ "T2\u0001",
+ // Non-ASCII whitespace is never trimmed (null on all versions)
+ "\u3000T2",
+ "T2\u3000",
// Full datetime: leading whitespace (valid on all versions — full trim applies)
" 2020-01-01T12:34:56",
"\t2020-01-01T12:34:56",
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org