This is an automated email from the ASF dual-hosted git repository.
sunchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new c152051895 fix: align string to timestamp parsing with Spark's segment
rules (#5682)
c152051895 is described below
commit c1520518953473b4430941ef9dbf86ebd6e400e2
Author: Peter Lee <[email protected]>
AuthorDate: Thu Sep 10 08:14:42 2026 +0800
fix: align string to timestamp parsing with Spark's segment rules (#5682)
* fix: align string to timestamp parsing with Spark's segment rules
Spark's SparkDateTimeUtils.parseTimestampString validates each segment
with isValidDigits: month, day, hour, minute and second take 1-2 digits,
the fraction after '.' may be empty, a timestamp year takes at most 6
digits (only stringToDate allows 7), and a zone id is only captured when
the scanner is inside the seconds or fraction segment.
Comet's regex table required exactly 2 digits and a non-empty fraction,
allowed 7-digit years, and stripped a zone suffix from any shape, so
'2020-1-1', '2020-01-01 12:34:5' and '2020-01-01 12:34:56.' returned
NULL (or raised under ANSI) while '2020-10-01Z' and
'0002020-01-01 00:00:00' were accepted. Relax the segment quantifiers,
cap the year at 6 digits for timestamp shapes, allow an empty fraction,
and only honour a stripped suffix when the remainder ends in a seconds
or fraction segment, for both TIMESTAMP and TIMESTAMP_NTZ. Previously
accepted inputs keep their exact values; CAST(... AS DATE) keeps its
7-digit years because date_parser is a separate port of stringToDate.
Closes #5674
Co-Authored-By: Claude Fable 5.1 <[email protected]>
* fix: restrict timestamp segments to ASCII digits
* fix: use ASCII digits throughout timestamp patterns
* fix: address timestamp parser review regressions and fuzz coverage
* Preserve Spark 4 leading-whitespace rejection with timestamp offsets
* fix: match Java trimming for timestamp zone separators
* fix: validate numeric timestamp offsets before normalization
---------
Co-authored-by: Claude Fable 5.1 <[email protected]>
---
.../user-guide/latest/compatibility/index.md | 3 +
.../spark-expr/benches/cast_string_to_timestamp.rs | 25 +-
native/spark-expr/src/conversion_funcs/string.rs | 555 ++++++++++++++++++---
.../org/apache/comet/CometNativeCastSuite.scala | 162 +++++-
4 files changed, 668 insertions(+), 77 deletions(-)
diff --git a/docs/source/user-guide/latest/compatibility/index.md
b/docs/source/user-guide/latest/compatibility/index.md
index 001ec87606..fd7c15ff3d 100644
--- a/docs/source/user-guide/latest/compatibility/index.md
+++ b/docs/source/user-guide/latest/compatibility/index.md
@@ -139,6 +139,9 @@ so users hunting an unexpected value have a single place to
check:
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)).
+- **Explicit positive timestamp years:** Spark accepts strings such as `+7528`
as the start
+ of that year, while Comet's native string-to-timestamp cast returns NULL in
non-ANSI mode
+ ([#5716](https://github.com/apache/datafusion-comet/issues/5716)).
- 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/benches/cast_string_to_timestamp.rs
b/native/spark-expr/benches/cast_string_to_timestamp.rs
index 3e83d78756..e28afb8272 100644
--- a/native/spark-expr/benches/cast_string_to_timestamp.rs
+++ b/native/spark-expr/benches/cast_string_to_timestamp.rs
@@ -30,11 +30,22 @@ const BATCH_SIZE: usize = 8192;
fn criterion_benchmark(c: &mut Criterion) {
let expr = Arc::new(Column::new("a", 0)) as Arc<dyn PhysicalExpr>;
- // Input shapes, chosen to cover each branch `timestamp_parser` can take:
the canonical
- // form, the fractional-second form, an offset suffix (which takes the
extract-offset
- // path), a date-only string, whitespace padding (the trim), and a mix
that includes
- // invalid values so the null path is measured too.
+ // Cover single-digit segments, empty fractions, rejected date-only zones,
canonical
+ // timestamps, microseconds, offset extraction, date-only input,
whitespace padding,
+ // and a mix containing invalid values so the null path is measured too.
let batches = [
+ (
+ "single_digit_segments",
+ create_batch(|i| format!("2020-{:02}-{}T1:2:3", i % 12 + 1, i % 9
+ 1)),
+ ),
+ (
+ "empty_fraction",
+ create_batch(|i| format!("2020-01-{:02}T12:34:56.", i % 28 + 1)),
+ ),
+ (
+ "date_only_zone",
+ create_batch(|i| format!("2020-01-{:02}Z", i % 28 + 1)),
+ ),
(
"canonical",
create_batch(|i| {
@@ -151,7 +162,7 @@ fn criterion_benchmark(c: &mut Criterion) {
for (name, batch) in &batches {
// ANSI raises on the first invalid value, so timing it
against a batch that is
// mostly invalid would measure the error path rather than the
parser.
- if mode == EvalMode::Ansi && *name == "mixed" {
+ if mode == EvalMode::Ansi && matches!(*name, "mixed" |
"date_only_zone") {
continue;
}
let cast = Cast::new(
@@ -169,8 +180,8 @@ fn criterion_benchmark(c: &mut Criterion) {
}
}
- // The Spark 4 path adds a leading-whitespace check for T-prefixed
time-only strings, so it
- // is measured separately on the inputs where that check can fire.
+ // Measure the cost of Spark 4's leading-whitespace/T-prefix check.
Neither padded nor
+ // mixed combines both conditions, so these inputs never take its
rejection branch.
let mut group =
c.benchmark_group("cast_string_to_timestamp/spark4_legacy");
for name in ["padded", "mixed"] {
let batch = &batches.iter().find(|(n, _)| *n == name).unwrap().1;
diff --git a/native/spark-expr/src/conversion_funcs/string.rs
b/native/spark-expr/src/conversion_funcs/string.rs
index e460d2dfc6..e56816aed3 100644
--- a/native/spark-expr/src/conversion_funcs/string.rs
+++ b/native/spark-expr/src/conversion_funcs/string.rs
@@ -1153,11 +1153,15 @@ fn parse_to_timestamp_info(
let hour = parts.next().map_or(0, |h| h.parse::<u32>().unwrap_or(0));
let minute = parts.next().map_or(0, |m| m.parse::<u32>().unwrap_or(0));
let second = parts.next().map_or(0, |s| s.parse::<u32>().unwrap_or(0));
- let microsecond = parts.next().map_or(0, |ms| {
- let ms = &ms[..ms.len().min(6)];
+ let microsecond = if let Some(ms) = parts.next() {
+ let Some(ms) = ms.get(..ms.len().min(6)) else {
+ return Ok(None);
+ };
let n = ms.len();
ms.parse::<u32>().unwrap_or(0) * 10u32.pow((6 - n) as u32)
- });
+ } else {
+ 0
+ };
let mut timestamp_info = TimeStampInfo::default();
@@ -1417,14 +1421,8 @@ fn timestamp_parser<T: TimeZone>(
// 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.
- // Check the raw (pre-trim) value for leading whitespace before any
T-time-only match.
- if is_spark4_plus
- && value.len() > value.trim_start().len()
- && (RE_TIME_ONLY_H.is_match(trimmed)
- || RE_TIME_ONLY_HM.is_match(trimmed)
- || RE_TIME_ONLY_HMS.is_match(trimmed)
- || RE_TIME_ONLY_HMSU.is_match(trimmed))
- {
+ // 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') {
return if eval_mode == EvalMode::Ansi {
Err(SparkError::InvalidInputInCastToDatetime {
value: value.to_string(),
@@ -1470,7 +1468,14 @@ fn timestamp_parser<T: TimeZone>(
if !has_direct_match {
if let Some((stripped, suffix_tz)) = extract_offset_suffix(value) {
- return timestamp_parser_with_tz(stripped, eval_mode, &suffix_tz);
+ // Spark applies Java String.trim to the zone, not Unicode
whitespace trimming.
+ let stripped = stripped.trim_end_matches(|c: char| c <= '\u{20}');
+ // A zone suffix is only meaningful after the seconds segment.
Otherwise fall
+ // through with the unstripped value, which no base pattern
matches, so it is
+ // reported as malformed (null, or CAST_INVALID_INPUT under ANSI)
like Spark does.
+ if ends_with_seconds_segment(stripped) {
+ return timestamp_parser_with_tz(stripped, eval_mode,
&suffix_tz);
+ }
}
}
@@ -1490,7 +1495,8 @@ fn timestamp_parser<T: TimeZone>(
/// "+HH:MM" -> same
/// (negative with '-' analogously)
///
-/// Hours must be 0–18 and minutes 0–59. A trailing colon ("+8:") is rejected.
+/// Hours must be 0–18 and minutes 0–59, with a maximum absolute offset of
18:00.
+/// A trailing colon ("+8:") is rejected.
fn parse_sign_offset(s: &str) -> Option<i32> {
if s.is_empty() {
return Some(0);
@@ -1500,14 +1506,16 @@ fn parse_sign_offset(s: &str) -> Option<i32> {
Some(&b'-') => (-1i32, &s[1..]),
_ => return None,
};
- if rest.is_empty() {
- return None; // lone '+' or '-'
+ // Validate before slicing: malformed date segments can reach this helper,
and a
+ // byte range such as rest[..2] must not split a non-ASCII digit's UTF-8
encoding.
+ if rest.is_empty() || !rest.bytes().all(|b| b.is_ascii_digit() || b ==
b':') {
+ return None;
}
let (h, m) = if let Some(colon_pos) = rest.find(':') {
let h_str = &rest[..colon_pos];
let m_str = &rest[colon_pos + 1..];
- if m_str.is_empty() {
- return None; // trailing colon: "+8:"
+ if !(1..=2).contains(&h_str.len()) || !(1..=2).contains(&m_str.len()) {
+ return None;
}
let h: i32 = h_str.parse().ok()?;
// Note: "+HH:MM:SS" (with seconds) is not handled; Spark accepts it
but it is rare.
@@ -1523,7 +1531,7 @@ fn parse_sign_offset(s: &str) -> Option<i32> {
_ => return None,
}
};
- if !(0..=18).contains(&h) || !(0..=59).contains(&m) {
+ if !(0..=18).contains(&h) || !(0..=59).contains(&m) || (h == 18 && m != 0)
{
return None;
}
Some(sign * (h * 3600 + m * 60))
@@ -1636,34 +1644,58 @@ fn extract_offset_suffix(value: &str) -> Option<(&str,
Tz)> {
type TimestampParsePattern<T> = (&'static Regex, fn(&str, &T) ->
SparkResult<Option<i64>>);
-// RE_YEAR allows only 4-6 digits (not 7) because a bare 7-digit string like
"0119704"
-// is ambiguous and Spark rejects it. The other patterns (RE_MONTH, RE_DAY,
etc.) keep
-// \d{4,7} because the `-` separator disambiguates the year portion, so
"0002020-01-01"
-// is validly year 2020 with leading zeros. date_parser's is_valid_digits also
allows up
-// to 7 year digits for the same reason.
-static RE_YEAR: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^-?\d{4,6}$").unwrap());
-static RE_MONTH: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^-?\d{4,7}-\d{2}$").unwrap());
-static RE_DAY: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^-?\d{4,7}-\d{2}-\d{2}$").unwrap());
+// These shapes transcribe the per-segment digit rules of Spark's
+// `SparkDateTimeUtils.parseTimestampString` (`isValidDigits`): the year takes
4-6 digits
+// (`maxDigitsYear = 6`, so "0002020-01-01" is malformed for a timestamp even
though
+// `stringToDate`, ported by `date_parser`, allows 7),
month/day/hour/minute/second take 1-2
+// digits each, and the fraction takes any number of digits including none
("12:34:56." is
+// valid), of which only the first six are kept. All digits must be ASCII,
matching Spark's
+// byte scanner and the numeric parsers used after shape recognition.
+// Keep the ASCII ranges: Unicode `\d` also costs substantially more to match
on valid input.
+static RE_YEAR: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^-?[0-9]{4,6}$").unwrap());
+static RE_MONTH: LazyLock<Regex> =
+ LazyLock::new(|| Regex::new(r"^-?[0-9]{4,6}-[0-9]{1,2}$").unwrap());
+static RE_DAY: LazyLock<Regex> =
+ LazyLock::new(||
Regex::new(r"^-?[0-9]{4,6}-[0-9]{1,2}-[0-9]{1,2}$").unwrap());
static RE_HOUR: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^-?\d{4,7}-\d{2}-\d{2}[T
]\d{1,2}$").unwrap());
-static RE_MINUTE: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^-?\d{4,7}-\d{2}-\d{2}[T
]\d{2}:\d{2}$").unwrap());
-static RE_SECOND: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^-?\d{4,7}-\d{2}-\d{2}[T
]\d{2}:\d{2}:\d{2}$").unwrap());
-static RE_MICROSECOND: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^-?\d{4,7}-\d{2}-\d{2}[T
]\d{2}:\d{2}:\d{2}\.\d+$").unwrap());
-static RE_TIME_ONLY_H: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^T\d{1,2}$").unwrap());
+ LazyLock::new(|| Regex::new(r"^-?[0-9]{4,6}-[0-9]{1,2}-[0-9]{1,2}[T
][0-9]{1,2}$").unwrap());
+static RE_MINUTE: LazyLock<Regex> = LazyLock::new(|| {
+ Regex::new(r"^-?[0-9]{4,6}-[0-9]{1,2}-[0-9]{1,2}[T
][0-9]{1,2}:[0-9]{1,2}$").unwrap()
+});
+static RE_SECOND: LazyLock<Regex> = LazyLock::new(|| {
+ Regex::new(r"^-?[0-9]{4,6}-[0-9]{1,2}-[0-9]{1,2}[T
][0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}$").unwrap()
+});
+static RE_MICROSECOND: LazyLock<Regex> = LazyLock::new(|| {
+ Regex::new(r"^-?[0-9]{4,6}-[0-9]{1,2}-[0-9]{1,2}[T
][0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}\.[0-9]*$")
+ .unwrap()
+});
+static RE_TIME_ONLY_H: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^T[0-9]{1,2}$").unwrap());
static RE_TIME_ONLY_HM: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^T\d{1,2}:\d{1,2}$").unwrap());
+ LazyLock::new(|| Regex::new(r"^T[0-9]{1,2}:[0-9]{1,2}$").unwrap());
static RE_TIME_ONLY_HMS: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^T\d{1,2}:\d{1,2}:\d{1,2}$").unwrap());
+ LazyLock::new(||
Regex::new(r"^T[0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}$").unwrap());
static RE_TIME_ONLY_HMSU: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^T\d{1,2}:\d{1,2}:\d{1,2}\.\d+$").unwrap());
-static RE_BARE_HM: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^\d{1,2}:\d{1,2}$").unwrap());
+ LazyLock::new(||
Regex::new(r"^T[0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}\.[0-9]*$").unwrap());
+static RE_BARE_HM: LazyLock<Regex> =
+ LazyLock::new(|| Regex::new(r"^[0-9]{1,2}:[0-9]{1,2}$").unwrap());
static RE_BARE_HMS: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^\d{1,2}:\d{1,2}:\d{1,2}$").unwrap());
+ LazyLock::new(||
Regex::new(r"^[0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}$").unwrap());
static RE_BARE_HMSU: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^\d{1,2}:\d{1,2}:\d{1,2}\.\d+$").unwrap());
+ LazyLock::new(||
Regex::new(r"^[0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}\.[0-9]*$").unwrap());
+
+/// Whether `value` (a datetime with any zone suffix already stripped) ends in
a seconds or
+/// fraction segment. Spark's `parseTimestampString` only captures a zone id
when its byte
+/// scanner hits a non-digit while inside those two segments, so a suffix such
as `Z`, `+05:30`
+/// or ` UTC` is legal after `hh:mm:ss` or `hh:mm:ss.f*` but makes a
date-only, hour-only or
+/// hour:minute value malformed ("2020-10-01Z" and "2020-01-01T12:34Z" are
both null).
+fn ends_with_seconds_segment(value: &str) -> bool {
+ RE_SECOND.is_match(value)
+ || RE_MICROSECOND.is_match(value)
+ || RE_TIME_ONLY_HMS.is_match(value)
+ || RE_TIME_ONLY_HMSU.is_match(value)
+ || RE_BARE_HMS.is_match(value)
+ || RE_BARE_HMSU.is_match(value)
+}
fn timestamp_parser_with_tz<T: TimeZone>(
value: &str,
@@ -1673,7 +1705,7 @@ fn timestamp_parser_with_tz<T: TimeZone>(
// Both T-separator and space-separator date-time forms are supported.
// Negative years are handled by get_timestamp_values detecting a leading
'-'.
let patterns: &[TimestampParsePattern<T>] = &[
- // Year only: 4-7 digits, optionally negative
+ // Year only: 4-6 digits, optionally negative
(
&RE_YEAR,
parse_str_to_year_timestamp as fn(&str, &T) ->
SparkResult<Option<i64>>,
@@ -1787,23 +1819,30 @@ fn timestamp_ntz_parser(
|| RE_SECOND.is_match(value)
|| RE_MICROSECOND.is_match(value);
- // If no direct match, try stripping a timezone suffix
+ // If no direct match, try stripping a timezone suffix. Spark only
recognises a zone after
+ // the seconds segment; a suffix anywhere else leaves the unstripped
value, which no base
+ // pattern matches, so the inner parser reports it as malformed.
let value_to_parse = if !has_direct_match {
- if let Some((stripped, _tz)) = extract_offset_suffix(value) {
- if !allow_time_zone {
- return if eval_mode == EvalMode::Ansi {
- Err(SparkError::InvalidInputInCastToDatetime {
- value: value.to_string(),
- from_type: "STRING".to_string(),
- to_type: "TIMESTAMP_NTZ".to_string(),
- })
- } else {
- Ok(None)
- };
+ match extract_offset_suffix(value) {
+ Some((stripped, _tz))
+ if ends_with_seconds_segment(
+ stripped.trim_end_matches(|c: char| c <= '\u{20}'),
+ ) =>
+ {
+ if !allow_time_zone {
+ return if eval_mode == EvalMode::Ansi {
+ Err(SparkError::InvalidInputInCastToDatetime {
+ value: value.to_string(),
+ from_type: "STRING".to_string(),
+ to_type: "TIMESTAMP_NTZ".to_string(),
+ })
+ } else {
+ Ok(None)
+ };
+ }
+ stripped.trim_end_matches(|c: char| c <= '\u{20}')
}
- stripped.trim_end()
- } else {
- value
+ _ => value,
}
} else {
value
@@ -1862,7 +1901,9 @@ fn parse_str_to_time_only_timestamp<T: TimeZone>(value:
&str, tz: &T) -> SparkRe
let ns: u32 = if let Some(dot) = dot_idx {
let frac = &sec_frac[dot + 1..];
// Interpret up to 6 digits as microseconds, padding with trailing
zeros.
- let trimmed = &frac[..frac.len().min(6)];
+ let Some(trimmed) = frac.get(..frac.len().min(6)) else {
+ return Ok(None);
+ };
let padded = format!("{:0<6}", trimmed);
padded.parse::<u32>().unwrap_or(0) * 1000
} else {
@@ -2549,13 +2590,24 @@ mod tests {
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.
- for ws_input in &[" T2:30", "\tT2:30", "\nT2:30", " T2", "\tT2",
"\nT2"] {
- assert!(
- timestamp_parser(ws_input, EvalMode::Legacy, tz, true)
- .unwrap()
- .is_none(),
- "'{ws_input}' should be null in Legacy mode on Spark 4.0+"
- );
+ for ws_input in &[
+ " T2:30",
+ "\tT2:30",
+ "\nT2:30",
+ " T2",
+ "\tT2",
+ "\nT2",
+ "\tT1:2:3 +08:00",
+ " T1:2:3.4 +08:00",
+ ] {
+ for mode in [EvalMode::Legacy, EvalMode::Try] {
+ assert!(
+ timestamp_parser(ws_input, mode, tz, true)
+ .unwrap()
+ .is_none(),
+ "'{ws_input}' should be null in {mode:?} mode on Spark
4.0+"
+ );
+ }
// In ANSI mode the same inputs must raise an error (not silently
return null).
assert!(
timestamp_parser(ws_input, EvalMode::Ansi, tz, true).is_err(),
@@ -2570,7 +2622,7 @@ mod tests {
);
}
// Without leading whitespace, these must be valid on all versions.
- for ok_input in &["T2:30", "T2"] {
+ for ok_input in &["T2:30", "T2", "T1:2:3 +08:00", "T1:2:3.4 +08:00"] {
assert!(
timestamp_parser(ok_input, EvalMode::Legacy, tz, true)
.unwrap()
@@ -2956,6 +3008,379 @@ mod tests {
);
}
+ // 2020-01-01T00:00:00Z, 2020-01-01T12:34:56Z and the same wall clock at
+05:30, in micros.
+ const JAN1_2020: i64 = 1577836800000000;
+ const JAN1_2020_123456: i64 = 1577882096000000;
+ const JAN1_2020_123456_PLUS_0530: i64 = 1577862296000000;
+
+ /// Inputs Spark's `parseTimestampString` accepts that the fixed 2-digit
shapes rejected:
+ /// 1-2 digit month/day/hour/minute/second, an empty fraction (also before
a zone), and
+ /// 6-digit years (issue #5674). Values are UTC micros, identical for
TIMESTAMP_NTZ.
+ const SPARK_SEGMENT_RULE_VALID: &[(&str, i64)] = &[
+ ("2020-10-1", 1_601_510_400_000_000),
+ ("2020-12-1", 1_606_780_800_000_000),
+ ("2020-1", JAN1_2020),
+ ("2020-1-1", JAN1_2020),
+ ("2020-1-1T1", JAN1_2020 + 3600 * 1_000_000),
+ ("2020-1-1 1:2", JAN1_2020 + 3720 * 1_000_000),
+ ("2020-01-01 12:34:5", JAN1_2020 + 45245 * 1_000_000),
+ ("2020-1-1T1:2:3.4", JAN1_2020 + 3723 * 1_000_000 + 400_000),
+ ("2020-01-01 12:34:56.", JAN1_2020_123456),
+ ("002020-01-01 00:00:00", JAN1_2020),
+ ];
+
+ /// Inputs Spark rejects: non-ASCII segment digits, a zone suffix anywhere
but after the
+ /// seconds segment, more than six year digits, and more than two digits
in any other segment.
+ const SPARK_SEGMENT_RULE_INVALID: &[&str] = &[
+ "2020-01-01 12:34:56.1٢٢٢",
+ "T1:2:3.1٢٢٢",
+ "2020-1-1T٢",
+ "2020-1-1T1:2:3.٢",
+ "٢020-1-1",
+ "2020-٢",
+ "2020-01-٢",
+ "2020-1\u{967}",
+ "2020-\u{967}1",
+ "2020-01-1\u{967}",
+ "2020-01-\u{967}1",
+ "2020-01-01 12:34:56 +08:000",
+ "2020-01-01 12:34:56 +008:00",
+ "2020-01-01 12:34:56 +18:01",
+ "2020-01-01 12:34:56 -18:01",
+ "2020-01-01 12:34:56+08:000",
+ "2020-01-01 12:34:56+008:00",
+ "2020-01-01 12:34:56+18:01",
+ "2020-01-01 12:34:56 UTC+08:000",
+ "2020-01-01 12:34:56 GMT+008:00",
+ "2020-01-01 12:34:56 UT+18:01",
+ "2020-01-01T1:٢",
+ "2020-01-01T1:2:٣",
+ "2020-1-1T1:2:3.٢Z",
+ "T٢",
+ "T1:٢",
+ "T1:2:٣",
+ "T1:2:3.٢",
+ "1:٢",
+ "1:2:٣",
+ "1:2:3.٢",
+ "2020Z",
+ "2020-10-01Z",
+ "2020-01-01+05:30",
+ "2020-01-01-08:00",
+ "2020-10-01 UTC",
+ "2020-01-01T12Z",
+ "2020-01-01 12 UTC",
+ "2020-01-01T12:34Z",
+ "2020-01-01 12:34 UTC",
+ "2020-01-01T12:34:Z",
+ "0002020-01-01",
+ "0002020-01-01 00:00:00",
+ "-0002020-01-01",
+ "2020-001-01",
+ "2020-01-001",
+ "2020-01-01T123",
+ "2020-01-01T12:345",
+ "2020-01-01T12:34:567",
+ ];
+
+ #[test]
+ #[cfg_attr(miri, ignore)]
+ fn timestamp_zone_whitespace_matches_java_trim() {
+ let tz = &Tz::from_str("UTC").unwrap();
+ for whitespace in [
+ " ", "\t", "\n", "\u{1}", "\u{b}", "\u{c}", "\u{7f}", "\u{a0}",
"\u{2009}", "\u{3000}",
+ ] {
+ let valid = whitespace.chars().all(|c| c <= '\u{20}');
+ for (suffix, offset) in [("+08:00", 28_800_000_000), ("UTC", 0),
("Z", 0)] {
+ for (fraction, micros) in [("", 0), (".123", 123_000)] {
+ let input = format!("2020-01-01
12:34:56{fraction}{whitespace}{suffix}");
+ for mode in [EvalMode::Legacy, EvalMode::Try,
EvalMode::Ansi] {
+ for spark4 in [false, true] {
+ for (result, expected) in [
+ (
+ timestamp_parser(&input, mode, tz, spark4),
+ JAN1_2020_123456 + micros - offset,
+ ),
+ (
+ timestamp_ntz_parser(&input, mode, true,
spark4),
+ JAN1_2020_123456 + micros,
+ ),
+ ] {
+ if valid {
+ assert_eq!(
+ result.unwrap(),
+ Some(expected),
+ "{input:?}, {mode:?}"
+ );
+ } else if mode == EvalMode::Ansi {
+ assert!(
+ matches!(
+ result,
+
Err(SparkError::InvalidInputInCastToDatetime { .. })
+ ),
+ "{input:?}"
+ );
+ } else {
+ assert_eq!(result.unwrap(), None,
"{input:?}, {mode:?}");
+ }
+ }
+ let no_zone = timestamp_ntz_parser(&input, mode,
false, spark4);
+ if mode == EvalMode::Ansi {
+ assert!(no_zone.is_err(), "{input:?}");
+ } else {
+ assert_eq!(no_zone.unwrap(), None,
"{input:?}");
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ #[test]
+ fn timestamp_numeric_offset_validation() {
+ for (input, seconds) in [
+ ("", 0),
+ ("+0", 0),
+ ("-00", 0),
+ ("+8", 28_800),
+ ("+08", 28_800),
+ ("+0800", 28_800),
+ ("+8:0", 28_800),
+ ("+08:0", 28_800),
+ ("+8:00", 28_800),
+ ("+17:59", 64_740),
+ ("+18:00", 64_800),
+ ("-18:00", -64_800),
+ ("+1800", 64_800),
+ ("-1800", -64_800),
+ ] {
+ assert_eq!(parse_sign_offset(input), Some(seconds), "{input:?}");
+ }
+ for input in [
+ "+08:000",
+ "+008:00",
+ "+18:01",
+ "-18:01",
+ "+18:59",
+ "+19",
+ "-1900",
+ "+8:",
+ "+1:+1",
+ "-1:-1",
+ "+1\u{967}",
+ "+\u{967}1",
+ "+1٢",
+ "++1",
+ ] {
+ assert_eq!(parse_sign_offset(input), None, "{input:?}");
+ }
+ }
+
+ #[test]
+ #[cfg_attr(miri, ignore)]
+ fn timestamp_parser_spark_segment_rules_test() {
+ let tz = &Tz::from_str("UTC").unwrap();
+ // Exercise the decoders without their regex gates: malformed UTF-8
boundaries
+ // must not panic even if the accepted patterns change later.
+ assert!(
+ parse_to_timestamp_info("2020-01-01 12:34:56.1٢٢٢", "microsecond")
+ .unwrap()
+ .is_none()
+ );
+ assert_eq!(
+ parse_str_to_time_only_timestamp("T1:2:3.1٢٢٢", tz).unwrap(),
+ None
+ );
+ for mode in [EvalMode::Legacy, EvalMode::Try, EvalMode::Ansi] {
+ assert_eq!(
+ timestamp_parser("2021-11-22 10:54:27 +08:00", mode, tz,
true).unwrap(),
+ Some(1_637_549_667_000_000)
+ );
+ assert_eq!(
+ timestamp_ntz_parser("2021-11-22 10:54:27 +08:00", mode, true,
true).unwrap(),
+ Some(1_637_578_467_000_000)
+ );
+ }
+ for &(input, expected) in SPARK_SEGMENT_RULE_VALID {
+ for eval_mode in [EvalMode::Legacy, EvalMode::Try, EvalMode::Ansi]
{
+ assert_eq!(
+ timestamp_parser(input, eval_mode, tz, true).unwrap(),
+ Some(expected),
+ "{input:?} in {eval_mode:?}"
+ );
+ }
+ }
+ // An empty fraction may still be followed by a zone.
+ assert_eq!(
+ timestamp_parser("2020-01-01 12:34:56.Z", EvalMode::Legacy, tz,
true).unwrap(),
+ Some(JAN1_2020_123456)
+ );
+ assert_eq!(
+ timestamp_parser("2020-01-01 12:34:56.+05:30", EvalMode::Legacy,
tz, true).unwrap(),
+ Some(JAN1_2020_123456_PLUS_0530)
+ );
+
+ // Time-only shapes may carry a zone after their seconds segment but
not before it.
+ for input in ["T12:34:56Z", "12:34:56+05:30", "T1:2:3.Z"] {
+ assert!(
+ timestamp_parser(input, EvalMode::Ansi, tz, true)
+ .unwrap()
+ .is_some(),
+ "{input:?}"
+ );
+ }
+ for input in
+ SPARK_SEGMENT_RULE_INVALID
+ .iter()
+ .copied()
+ .chain(["T12Z", "12:34Z", "T12:34 UTC"])
+ {
+ for eval_mode in [EvalMode::Legacy, EvalMode::Try] {
+ assert_eq!(
+ timestamp_parser(input, eval_mode, tz, true).unwrap(),
+ None,
+ "{input:?} in {eval_mode:?}"
+ );
+ }
+ assert!(
+ timestamp_parser(input, EvalMode::Ansi, tz, true).is_err(),
+ "{input:?} in Ansi"
+ );
+ }
+
+ // Shapes that were already accepted keep their exact values.
+ let la_offset = 8 * 3600 * 1_000_000; // America/Los_Angeles is UTC-8
in January
+ for (input, expected) in [
+ ("2020-01-01", JAN1_2020),
+ ("2020-01-01 12:34:56", JAN1_2020_123456),
+ ("2020-01-01T12:34:56.123456", JAN1_2020_123456 + 123456),
+ ("2020-01-01T12:34:56Z", JAN1_2020_123456),
+ ("2020-01-01T12:34:56.123Z", JAN1_2020_123456 + 123000),
+ ("2020-01-01T12:34:56+05:30", JAN1_2020_123456_PLUS_0530),
+ ("2020-01-01T12:34:56 UTC", JAN1_2020_123456),
+ ("2020-01-01T12:34:56 UTC+5:30", JAN1_2020_123456_PLUS_0530),
+ (
+ "2020-01-01T12:34:56 America/Los_Angeles",
+ JAN1_2020_123456 + la_offset,
+ ),
+ ("-0001-01-01T12:34:56", -62198709904000000),
+ ] {
+ for eval_mode in [EvalMode::Legacy, EvalMode::Try, EvalMode::Ansi]
{
+ assert_eq!(
+ timestamp_parser(input, eval_mode, tz, true).unwrap(),
+ Some(expected),
+ "{input:?} in {eval_mode:?}"
+ );
+ }
+ }
+
+ // `date_parser` ports `stringToDate`, whose `maxDigitsYear` is 7, so
a date cast keeps
+ // accepting the 7-digit year that a timestamp cast rejects.
+ for eval_mode in [EvalMode::Legacy, EvalMode::Try, EvalMode::Ansi] {
+ assert_eq!(
+ date_parser("0002020-01-01", eval_mode).unwrap(),
+ Some(18262)
+ );
+ }
+ }
+
+ #[test]
+ #[cfg_attr(miri, ignore)]
+ fn timestamp_ntz_parser_spark_segment_rules_test() {
+ for allow_time_zone in [true, false] {
+ for &(input, expected) in SPARK_SEGMENT_RULE_VALID {
+ for eval_mode in [EvalMode::Legacy, EvalMode::Try,
EvalMode::Ansi] {
+ assert_eq!(
+ timestamp_ntz_parser(input, eval_mode,
allow_time_zone, false).unwrap(),
+ Some(expected),
+ "{input:?} in {eval_mode:?},
allow_time_zone={allow_time_zone}"
+ );
+ }
+ }
+ for &input in SPARK_SEGMENT_RULE_INVALID {
+ for eval_mode in [EvalMode::Legacy, EvalMode::Try] {
+ assert_eq!(
+ timestamp_ntz_parser(input, eval_mode,
allow_time_zone, false).unwrap(),
+ None,
+ "{input:?} in {eval_mode:?},
allow_time_zone={allow_time_zone}"
+ );
+ }
+ assert!(
+ timestamp_ntz_parser(input, EvalMode::Ansi,
allow_time_zone, false).is_err(),
+ "{input:?} in Ansi, allow_time_zone={allow_time_zone}"
+ );
+ }
+ }
+ // A zone after an empty fraction is discarded when allowed and
rejected otherwise.
+ assert_eq!(
+ timestamp_ntz_parser("2020-01-01 12:34:56.Z", EvalMode::Legacy,
true, false).unwrap(),
+ Some(JAN1_2020_123456)
+ );
+ assert_eq!(
+ timestamp_ntz_parser("2020-01-01 12:34:56.Z", EvalMode::Legacy,
false, false).unwrap(),
+ None
+ );
+ assert!(
+ timestamp_ntz_parser("2020-01-01 12:34:56.Z", EvalMode::Ansi,
false, false).is_err()
+ );
+ }
+
+ #[test]
+ #[cfg_attr(miri, ignore)]
+ fn test_cast_string_to_timestamp_spark_segment_rules_array() {
+ // The reproducer from issue #5674, through the batch entry points.
+ let inputs = vec![
+ Some("2020-1-1"),
+ Some("2020-01-01 12:34:5"),
+ Some("2020-01-01 12:34:56."),
+ Some("2020-10-01Z"),
+ Some("0002020-01-01 00:00:00"),
+ ];
+ let expected = [
+ Some(JAN1_2020),
+ Some(JAN1_2020 + 45245 * 1_000_000),
+ Some(JAN1_2020_123456),
+ None,
+ None,
+ ];
+ let array: ArrayRef = Arc::new(StringArray::from(inputs));
+ let to_type = DataType::Timestamp(TimeUnit::Microsecond,
Some("UTC".into()));
+
+ let tz_result =
+ cast_string_to_timestamp(&array, &to_type, EvalMode::Legacy,
"UTC", true).unwrap();
+ let ntz_result =
+ cast_string_to_timestamp_ntz(&array, EvalMode::Legacy, true,
false).unwrap();
+ for result in [&tz_result, &ntz_result] {
+ let result = result
+ .as_any()
+ .downcast_ref::<PrimitiveArray<TimestampMicrosecondType>>()
+ .unwrap();
+ let actual: Vec<Option<i64>> = result.iter().collect();
+ assert_eq!(actual, expected);
+ }
+
+ // Under ANSI the first malformed row fails the batch and names the
raw input.
+ let tz_err =
+ cast_string_to_timestamp(&array, &to_type, EvalMode::Ansi, "UTC",
true).unwrap_err();
+ let ntz_err =
+ cast_string_to_timestamp_ntz(&array, EvalMode::Ansi, true,
false).unwrap_err();
+ for (err, expected_type) in [(tz_err, "TIMESTAMP"), (ntz_err,
"TIMESTAMP_NTZ")] {
+ match err {
+ SparkError::InvalidInputInCastToDatetime {
+ value,
+ from_type,
+ to_type,
+ } => {
+ assert_eq!(value, "2020-10-01Z");
+ assert_eq!(from_type, "STRING");
+ assert_eq!(to_type, expected_type);
+ }
+ other => panic!("Expected InvalidInputInCastToDatetime, got
{other:?}"),
+ }
+ }
+ }
+
/// Asserts every date parses to null in legacy and try mode. When
`expect_ansi_error` is set,
/// ANSI mode must raise CAST_INVALID_INPUT; otherwise ANSI mode must also
return null.
fn assert_dates(dates: &[&str], expect_ansi_error: bool) {
diff --git a/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
b/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
index 70eb654410..2a2854ece3 100644
--- a/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
@@ -89,7 +89,7 @@ class CometNativeCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
private val datePattern = "0123456789/" + whitespaceChars
- private val timestampPattern = "0123456789/:T" + whitespaceChars
+ private val timestampPattern = "0123456789/:T-.+Z" + whitespaceChars
lazy val usingParquetExecWithIncompatTypes: Boolean =
hasUnsignedSmallIntSafetyCheck(conf)
@@ -1438,10 +1438,12 @@ class CometNativeCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
test("cast StringType to TimestampType") {
withSQLConf((SQLConf.SESSION_LOCAL_TIMEZONE.key, "UTC")) {
- val values = Seq("2020-01-01T12:34:56.123456", "T2") ++
gen.generateStrings(
- dataSize,
- timestampPattern,
- 8)
+ // Spark accepts explicit positive years; Comet does not yet (#5716).
+ // Keep the wider alphabet, excluding only the known bare-year mismatch.
+ val fuzzValues = gen
+ .generateStrings(dataSize, timestampPattern, 8)
+ .filterNot(_.trim.matches("\\+[0-9]{4,6}"))
+ val values = Seq("2020-01-01T12:34:56.123456", "T2") ++ fuzzValues
castTest(values.toDF("a"), DataTypes.TimestampType)
}
}
@@ -1541,6 +1543,17 @@ class CometNativeCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ test("cast StringType to TimestampType - time-only offset leading
whitespace") {
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC") {
+ // One Parquet-backed column value per query exercises each input in
Legacy, TRY and ANSI.
+ // Spark 4 rejects the leading whitespace; Spark 3.5 accepts it. NTZ
rejects time-only input.
+ Seq("\tT1:2:3 +08:00", " T1:2:3.4 +08:00", "T1:2:3 +08:00").foreach {
value =>
+ castTimestampTest(Seq(value).toDF("a"), DataTypes.TimestampType,
assertNative = true)
+ castTimestampTest(Seq(value).toDF("a"), DataTypes.TimestampNTZType,
assertNative = true)
+ }
+ }
+ }
+
Seq(DataTypes.TimestampType, DataTypes.TimestampNTZType).foreach { toType =>
test(s"cast StringType to $toType - out-of-range years") {
withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC") {
@@ -1579,6 +1592,145 @@ class CometNativeCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ // Spark's SparkDateTimeUtils.parseTimestampString validates each segment
with isValidDigits:
+ // month/day/hour/minute/second take 1-2 digits, the fraction may be empty,
a timestamp year
+ // takes at most 6 digits (only a date takes 7), and a zone id is only
recognised after the
+ // seconds segment. Keep explicit cases as well as fuzz coverage for these
segment boundaries.
+ private val sparkSegmentRuleTimestamps = Seq(
+ // 1-2 digit segments
+ "2020-1",
+ "2020-1-1",
+ "2020-10-1",
+ "2020-12-1",
+ "-0001-01-01T12:34:56",
+ "2021-11-22 10:54:27 +08:00",
+ "2020-01-01 12:34:56 Z",
+ "2020-01-01 12:34:56\t+08:00",
+ "2020-01-01 12:34:56\n+08:00",
+ "2020-01-01 12:34:56.123 +08:00",
+ "2020-01-01 12:34:56. +08:00",
+ "2020-01-01 12:34:56 +08:00",
+ "2020-01-01 12:34:56 -08:00",
+ "2020-01-01 12:34:56 Europe/Moscow",
+ "-0001-01-01T12:34:56 +08:00",
+ "-0001-01-01T12:34:56-08:00",
+ "2020-1-1T1",
+ "2020-1-1 1:2",
+ "2020-01-01 12:34:5",
+ "2020-1-1T1:2:3.4",
+ // empty fraction, alone and before a zone
+ "2020-01-01 12:34:56.",
+ "2020-01-01 12:34:56.Z",
+ // 6-digit year is the timestamp maximum
+ "002020-01-01 00:00:00")
+
+ private val sparkSegmentRuleMalformedTimestamps = Seq(
+ "-0002020-01-01",
+ "2020-01-01 12:34:56.1٢٢٢",
+ "T1:2:3.1٢٢٢",
+ // Spark's scanner only accepts ASCII digits in timestamp segments
+ "٢020-1-1",
+ "2020-٢",
+ "2020-01-٢",
+ "2020-1-1T٢",
+ "2020-01-01T1:٢",
+ "2020-01-01T1:2:٣",
+ "2020-1-1T1:2:3.٢",
+ "2020-1-1T1:2:3.٢Z",
+ "T٢",
+ "T1:٢",
+ "T1:2:٣",
+ "T1:2:3.٢",
+ "1:٢",
+ "1:2:٣",
+ "1:2:3.٢",
+ // zone suffix before the seconds segment
+ "2020-01-01 12:34 +08:00",
+ "2020-01-01 12 +08:00",
+ "2020-01-01 +08:00",
+ "2020-01-01 Z",
+ "2020-01 +08:00",
+ "2020 +08:00",
+ "2020Z",
+ "2020-10-01Z",
+ "2020-01-01+05:30",
+ "2020-01-01-08:00",
+ "2020-10-01 UTC",
+ "2020-01-01T12Z",
+ "2020-01-01T12:34Z",
+ "2020-01-01 12:34 UTC",
+ "2020-01-01T12:34:Z",
+ // 7-digit year
+ "0002020-01-01",
+ "0002020-01-01 00:00:00",
+ // 3-digit segments
+ "2020-001-01",
+ "2020-01-01T12:345")
+
+ test("cast StringType to TimestampType - Spark segment rules") {
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC") {
+ castTimestampTest(
+ sparkSegmentRuleTimestamps.toDF("a"),
+ DataTypes.TimestampType,
+ assertNative = true)
+ // One row per query so that every malformed value is checked under ANSI
mode rather
+ // than only the first row that fails a batch.
+ sparkSegmentRuleMalformedTimestamps.foreach { value =>
+ castTimestampTest(Seq(value).toDF("a"), DataTypes.TimestampType,
assertNative = true)
+ }
+ }
+ }
+
+ test("cast StringType to TimestampNTZType - Spark segment rules") {
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC") {
+ castTimestampTest(
+ sparkSegmentRuleTimestamps.toDF("a"),
+ DataTypes.TimestampNTZType,
+ assertNative = true)
+ sparkSegmentRuleMalformedTimestamps.foreach { value =>
+ castTimestampTest(Seq(value).toDF("a"), DataTypes.TimestampNTZType,
assertNative = true)
+ }
+ }
+ }
+
+ test("cast StringType to TimestampType/TimestampNTZType - whitespace before
zone suffix") {
+ // Zone names use Java String.trim (<= U+0020), not Unicode whitespace
trimming.
+ val values = Seq(0x01, 0x0b, 0x0c, 0x1f, 0x7f, 0x00a0, 0x2009, 0x3000)
+ .map(ws => s"2020-01-01 12:34:56${ws.toChar}+08:00") ++ Seq(
+ "2020-01-01 12:34:56\u00a0UTC",
+ "2020-01-01 12:34:56\u00a0Z",
+ "2020-01-01 12:34:56.123\u00a0+08:00")
+ for (tz <- Seq("UTC", "America/Los_Angeles")) {
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> tz) {
+ // One Parquet-backed value at a time checks every invalid input in
ANSI as well.
+ for (value <- values;
+ dataType <- Seq(DataTypes.TimestampType,
DataTypes.TimestampNTZType)) {
+ castTimestampTest(Seq(value).toDF("a"), dataType, assertNative =
true)
+ }
+ }
+ }
+ }
+
+ test("cast StringType to TimestampType/TimestampNTZType - numeric offset
validation") {
+ val malformed = Seq(
+ "2020-1\u0967",
+ "2020-\u09671",
+ "2020-01-1\u0967",
+ "2020-01-\u09671") ++ Seq("+08:000", "+008:00", "+18:01", "-18:01",
"+1:+1", "+1\u0967")
+ .flatMap(offset => Seq(s"2020-01-01 12:34:56 $offset", s"2020-01-01
12:34:56$offset"))
+ val valid = Seq("+08:00", "+8:0", "+0800", "+17:59", "+18:00", "-18:00")
+ .map(offset => s"2020-01-01 12:34:56 $offset")
+ for (tz <- Seq("UTC", "America/Los_Angeles")) {
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> tz) {
+ // Each Parquet-backed value is checked separately in legacy, TRY and
ANSI modes.
+ for (value <- malformed ++ valid;
+ dataType <- Seq(DataTypes.TimestampType,
DataTypes.TimestampNTZType)) {
+ castTimestampTest(Seq(value).toDF("a"), dataType, assertNative =
true)
+ }
+ }
+ }
+ }
+
// CAST from BinaryType
test("cast BinaryType to StringType") {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
