peterxcli commented on code in PR #5682:
URL: https://github.com/apache/datafusion-comet/pull/5682#discussion_r3938998584
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -1636,34 +1641,52 @@ 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.
+// 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.
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());
+static RE_MONTH: LazyLock<Regex> = LazyLock::new(||
Regex::new(r"^-?\d{4,6}-\d{1,2}$").unwrap());
Review Comment:
Fixed in 0a1fc76a0. All numeric classes in the timestamp patterns now use
`[0-9]`, including years, the hour-only shape, fractions, and time-only forms.
The shared Rust and Parquet-backed Spark fixtures include both `2020-1-1T٢` and
`2020-1-1T1:2:3.٢`, plus a zoned fraction and Unicode year/time-only cases.
Both timestamp types are checked in Legacy, TRY, and ANSI modes, with separate
Spark queries for each malformed input.
The expanded parser tests failed against bcd380371 on `2020-1-1T٢` for both
TIMESTAMP and TIMESTAMP_NTZ before the fix. Afterward, all three focused Rust
segment-rule tests passed. `make core` succeeded, and the two focused
`CometNativeCastSuite` segment-rule tests passed locally on Spark 4.1.3 (2
tests, 0 failures). Formatting, Scalastyle, and `git diff --check` also passed.
--
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]
