peterxcli commented on code in PR #5682:
URL: https://github.com/apache/datafusion-comet/pull/5682#discussion_r3937363845
##########
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 bcd380371. The newly broadened timestamp segment classes now use
ASCII digit ranges, and the Spark regression covers Unicode month, day, minute,
and second values through Parquet-backed column input for both TIMESTAMP and
TIMESTAMP_NTZ across Legacy/Try/ANSI behavior. Locally, the three focused Rust
segment-rule tests, cargo fmt --check, git diff --check, and ./mvnw
spotless:check pass. The local native JNI link did not finish, so I left the
full Spark-suite validation to CI.
--
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]
