parthchandra commented on code in PR #5130:
URL: https://github.com/apache/datafusion-comet/pull/5130#discussion_r3730698827
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -1619,87 +1574,311 @@ 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.
+/// The shapes are mutually exclusive, so at most one can apply to any given
string.
+#[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 equivalent regular expression for 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 below 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 portion, so "0002020-01-01" is validly year
2020 with leading
+ /// zeros. `date_parser`'s `is_valid_digits` allows up to 7 year digits
for the same reason.
+ 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+$",
+ }
+ }
+
+ /// True for the shapes that carry no date component: `T12`, `T12:34`,
`12:34`, ...
+ fn is_time_only(self) -> bool {
+ !matches!(
+ self,
+ Self::Year
+ | Self::Month
+ | Self::Day
+ | Self::Hour
+ | Self::Minute
+ | Self::Second
+ | Self::Microsecond
+ )
+ }
+
+ /// True for the `T`-prefixed time-only shapes only, which Spark 4.0+
rejects when the
+ /// raw value has leading whitespace.
+ fn is_t_time_only(self) -> bool {
+ matches!(
+ self,
+ Self::TimeOnlyH | Self::TimeOnlyHm | Self::TimeOnlyHms |
Self::TimeOnlyHmsu
+ )
+ }
+}
+
+static TIMESTAMP_PATTERN_SET: LazyLock<RegexSet> = LazyLock::new(|| {
+
RegexSet::new(TimestampPattern::ALL.map(TimestampPattern::regex_str)).unwrap()
+});
+
+/// Returns the shape `value` has, or `None` when it matches none of them.
+///
+/// ASCII input - effectively all real data - is classified by a single
left-to-right byte
+/// scan. Non-ASCII input falls back to a single `RegexSet` pass, which
reports every
+/// matching pattern in one search of the haystack; the lowest matching index
is taken so
+/// that the result is the same first-match-wins answer the ASCII scan gives.
+fn classify_timestamp_pattern(value: &str) -> Option<TimestampPattern> {
+ if value.is_ascii() {
+ classify_ascii_timestamp_pattern(value.as_bytes())
+ } else {
+ TIMESTAMP_PATTERN_SET
+ .matches(value)
+ .iter()
+ .next()
+ .map(|i| TimestampPattern::ALL[i])
+ }
+}
+
+/// Number of leading ASCII digits in `bytes`.
+fn digit_run(bytes: &[u8]) -> usize {
+ bytes
+ .iter()
+ .position(|b| !b.is_ascii_digit())
+ .unwrap_or(bytes.len())
+}
+
+/// True when `bytes` is a non-empty run of ASCII digits, i.e. a `\.\d+`
fraction tail.
+fn all_digits(bytes: &[u8]) -> bool {
+ !bytes.is_empty() && digit_run(bytes) == bytes.len()
+}
+
+/// ASCII-only equivalent of matching `value` against each
[`TimestampPattern::regex_str`] in
+/// order.
+fn classify_ascii_timestamp_pattern(bytes: &[u8]) -> Option<TimestampPattern> {
+ // `T`-prefixed time-only shapes.
+ if let [b'T', rest @ ..] = bytes {
+ return classify_ascii_time(rest, true);
+ }
+
+ let (negative, rest) = match bytes.split_first() {
+ Some((b'-', rest)) => (true, rest),
+ Some(_) => (false, bytes),
+ None => return None,
+ };
+
+ let digits = digit_run(rest);
+ let after_digits = &rest[digits..];
+ if after_digits.is_empty() {
+ // Year only.
+ return (4..=6).contains(&digits).then_some(TimestampPattern::Year);
+ }
+ match after_digits[0] {
+ // A year of 4-7 digits followed by the date separator.
+ b'-' if (4..=7).contains(&digits) =>
classify_ascii_date_tail(&after_digits[1..]),
+ // Bare time-only shapes take a 1-2 digit hour and no sign.
+ b':' if !negative && (1..=2).contains(&digits) =>
classify_ascii_time(rest, false),
+ _ => None,
+ }
+}
+
+/// Classifies `\d{1,2}(:\d{1,2}(:\d{1,2}(\.\d+)?)?)?`, the time-only shapes.
A bare hour
+/// with no minutes is only a shape when the `T` prefix was present.
+fn classify_ascii_time(bytes: &[u8], t_prefixed: bool) ->
Option<TimestampPattern> {
+ let hour = digit_run(bytes);
+ if !(1..=2).contains(&hour) {
+ return None;
+ }
+ let bytes = &bytes[hour..];
+ if bytes.is_empty() {
+ return t_prefixed.then_some(TimestampPattern::TimeOnlyH);
+ }
+ let [b':', bytes @ ..] = bytes else {
+ return None;
+ };
+
+ let minute = digit_run(bytes);
+ if !(1..=2).contains(&minute) {
+ return None;
+ }
+ let bytes = &bytes[minute..];
+ if bytes.is_empty() {
+ return Some(if t_prefixed {
+ TimestampPattern::TimeOnlyHm
+ } else {
+ TimestampPattern::BareHm
+ });
+ }
+ let [b':', bytes @ ..] = bytes else {
+ return None;
+ };
+
+ let second = digit_run(bytes);
+ if !(1..=2).contains(&second) {
+ return None;
+ }
+ let bytes = &bytes[second..];
+ if bytes.is_empty() {
+ return Some(if t_prefixed {
+ TimestampPattern::TimeOnlyHms
+ } else {
+ TimestampPattern::BareHms
+ });
+ }
+ let [b'.', fraction @ ..] = bytes else {
+ return None;
+ };
+ all_digits(fraction).then_some(if t_prefixed {
+ TimestampPattern::TimeOnlyHmsu
+ } else {
+ TimestampPattern::BareHmsu
+ })
+}
+
+/// Classifies `\d{2}(-\d{2}([T ]\d{1,2}(:\d{2}(:\d{2}(\.\d+)?)?)?)?)?`, the
part of a
+/// date-time shape that follows the year and its `-` separator. Note the
asymmetry the
+/// patterns encode: the hour is 1-2 digits when it ends the string, but
exactly 2 digits
+/// once a minute follows.
+fn classify_ascii_date_tail(bytes: &[u8]) -> Option<TimestampPattern> {
+ if digit_run(bytes) != 2 {
+ return None;
+ }
+ let bytes = &bytes[2..];
+ if bytes.is_empty() {
+ return Some(TimestampPattern::Month);
+ }
+ let [b'-', bytes @ ..] = bytes else {
+ return None;
+ };
+
+ if digit_run(bytes) != 2 {
+ return None;
+ }
+ let bytes = &bytes[2..];
+ if bytes.is_empty() {
+ return Some(TimestampPattern::Day);
+ }
+ let ([b'T', bytes @ ..] | [b' ', bytes @ ..]) = bytes else {
+ return None;
+ };
+
+ let hour = digit_run(bytes);
+ if !(1..=2).contains(&hour) {
+ return None;
+ }
+ let after_hour = &bytes[hour..];
+ if after_hour.is_empty() {
+ return Some(TimestampPattern::Hour);
+ }
+ // Everything past the hour requires a two-digit hour followed by a colon.
+ if hour != 2 {
+ return None;
+ }
+ let [b':', bytes @ ..] = after_hour else {
+ return None;
+ };
+
+ if digit_run(bytes) != 2 {
+ return None;
+ }
+ let bytes = &bytes[2..];
+ if bytes.is_empty() {
+ return Some(TimestampPattern::Minute);
+ }
+ let [b':', bytes @ ..] = bytes else {
+ return None;
+ };
+
+ if digit_run(bytes) != 2 {
+ return None;
+ }
+ let bytes = &bytes[2..];
+ if bytes.is_empty() {
+ return Some(TimestampPattern::Second);
+ }
+ let [b'.', fraction @ ..] = bytes else {
+ return None;
+ };
+ all_digits(fraction).then_some(TimestampPattern::Microsecond)
+}
fn timestamp_parser_with_tz<T: TimeZone>(
value: &str,
eval_mode: EvalMode,
tz: &T,
) -> SparkResult<Option<i64>> {
- // 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
- (
- &RE_YEAR,
- parse_str_to_year_timestamp as fn(&str, &T) ->
SparkResult<Option<i64>>,
- ),
- // Year-month
- (&RE_MONTH, parse_str_to_month_timestamp),
- // Year-month-day
- (&RE_DAY, parse_str_to_day_timestamp),
- // Date T-or-space hour (1 or 2 digits)
- (&RE_HOUR, parse_str_to_hour_timestamp),
- // Date T-or-space hour:minute
- (&RE_MINUTE, parse_str_to_minute_timestamp),
- // Date T-or-space hour:minute:second
- (&RE_SECOND, parse_str_to_second_timestamp),
- // Date T-or-space hour:minute:second.fraction
- (&RE_MICROSECOND, parse_str_to_microsecond_timestamp),
- // Time-only: T hour (1 or 2 digits, no colon)
- (&RE_TIME_ONLY_H, parse_str_to_time_only_timestamp),
- // Time-only: T hour:minute
- (&RE_TIME_ONLY_HM, parse_str_to_time_only_timestamp),
- // Time-only: T hour:minute:second
- (&RE_TIME_ONLY_HMS, parse_str_to_time_only_timestamp),
- // Time-only: T hour:minute:second.fraction
- (&RE_TIME_ONLY_HMSU, parse_str_to_time_only_timestamp),
- // Bare time-only: hour:minute (without T prefix)
- (&RE_BARE_HM, parse_str_to_time_only_timestamp),
- // Bare time-only: hour:minute:second
- (&RE_BARE_HMS, parse_str_to_time_only_timestamp),
- // Bare time-only: hour:minute:second.fraction
- (&RE_BARE_HMSU, parse_str_to_time_only_timestamp),
- ];
-
- let mut timestamp = None;
+ parse_timestamp_pattern(value, classify_timestamp_pattern(value),
eval_mode, tz)
+}
- // Iterate through patterns and try matching
- for (pattern, parse_func) in patterns {
- if pattern.is_match(value) {
- timestamp = parse_func(value, tz)?;
- break;
- }
- }
+/// Parses `value` according to the shape already determined for it by
+/// [`classify_timestamp_pattern`].
+///
+/// Both T-separator and space-separator date-time forms are supported.
Negative years are
+/// handled by `get_timestamp_values` detecting a leading '-'.
+fn parse_timestamp_pattern<T: TimeZone>(
+ value: &str,
+ pattern: Option<TimestampPattern>,
+ eval_mode: EvalMode,
+ tz: &T,
+) -> SparkResult<Option<i64>> {
+ let timestamp = match pattern {
+ Some(TimestampPattern::Year) => get_timestamp_values(value, "year",
tz)?,
+ Some(TimestampPattern::Month) => get_timestamp_values(value, "month",
tz)?,
+ Some(TimestampPattern::Day) => get_timestamp_values(value, "day", tz)?,
+ Some(TimestampPattern::Hour) => get_timestamp_values(value, "hour",
tz)?,
+ Some(TimestampPattern::Minute) => get_timestamp_values(value,
"minute", tz)?,
+ Some(TimestampPattern::Second) => get_timestamp_values(value,
"second", tz)?,
+ Some(TimestampPattern::Microsecond) => get_timestamp_values(value,
"microsecond", tz)?,
+ Some(_) => parse_str_to_time_only_timestamp(value, tz)?,
Review Comment:
Not a blocker, but this could be fragile if someone adds a new variant that
is not time-only to TimestampPattern. We'd fall thru to the catch-all and
produce wrong output.
--
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]
