peterxcli commented on code in PR #5130:
URL: https://github.com/apache/datafusion-comet/pull/5130#discussion_r3689946040
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -1619,87 +1574,307 @@ fn extract_offset_suffix(value: &str) -> Option<(&str,
timezone::Tz)> {
None
}
-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());
-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());
-static RE_TIME_ONLY_HM: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^T\d{1,2}:\d{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());
-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());
-static RE_BARE_HMS: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^\d{1,2}:\d{1,2}:\d{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());
+/// The timestamp string shapes the parser recognises, listed in the order
they are matched.
Review Comment:
I think this would be better to match the type and corresponding regex line
by line but still lazy initialising one `RegexSet`
```rust
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
enum TimestampPattern {
Year,
Month,
Day,
Hour,
Minute,
Second,
Microsecond,
TimeOnlyH,
TimeOnlyHm,
TimeOnlyHms,
TimeOnlyHmsu,
BareHm,
BareHms,
BareHmsu,
}
impl TimestampPattern {
/// Every shape, in the order they are matched. First match wins, so the
order matters.
const ALL: [TimestampPattern; 14] = [
Self::Year,
Self::Month,
Self::Day,
Self::Hour,
Self::Minute,
Self::Second,
Self::Microsecond,
Self::TimeOnlyH,
Self::TimeOnlyHm,
Self::TimeOnlyHms,
Self::TimeOnlyHmsu,
Self::BareHm,
Self::BareHms,
Self::BareHmsu,
];
/// The regex equivalent of this shape.
///
/// Only used for the rare non-ASCII input, where the Unicode-aware `\d`
class accepts
/// digits (e.g. Arabic-Indic) that the ASCII classifier does not.
///
/// `Year` allows only 4-6 digits (not 7) because a bare 7-digit string
like "0119704"
/// is ambiguous and Spark rejects it. The others keep `\d{4,7}` because
the `-`
/// separator disambiguates the year, so "0002020-01-01" is validly year
2020 with
/// leading zeros.
fn regex_str(self) -> &'static str {
match self {
Self::Year => r"^-?\d{4,6}$",
Self::Month => r"^-?\d{4,7}-\d{2}$",
Self::Day => r"^-?\d{4,7}-\d{2}-\d{2}$",
Self::Hour => r"^-?\d{4,7}-\d{2}-\d{2}[T ]\d{1,2}$",
Self::Minute => r"^-?\d{4,7}-\d{2}-\d{2}[T ]\d{2}:\d{2}$",
Self::Second => r"^-?\d{4,7}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}$",
Self::Microsecond => r"^-?\d{4,7}-\d{2}-\d{2}[T
]\d{2}:\d{2}:\d{2}\.\d+$",
Self::TimeOnlyH => r"^T\d{1,2}$",
Self::TimeOnlyHm => r"^T\d{1,2}:\d{1,2}$",
Self::TimeOnlyHms => r"^T\d{1,2}:\d{1,2}:\d{1,2}$",
Self::TimeOnlyHmsu => r"^T\d{1,2}:\d{1,2}:\d{1,2}\.\d+$",
Self::BareHm => r"^\d{1,2}:\d{1,2}$",
Self::BareHms => r"^\d{1,2}:\d{1,2}:\d{1,2}$",
Self::BareHmsu => r"^\d{1,2}:\d{1,2}:\d{1,2}\.\d+$",
}
}
}
static TIMESTAMP_PATTERN_SET: LazyLock<RegexSet> = LazyLock::new(|| {
RegexSet::new(TimestampPattern::ALL.map(TimestampPattern::regex_str)).unwrap()
});
```
--
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]
