Fixes after merge

- do not pin the `proc-macro` version to avoid conflicts in CubeStore,
- move forked `string_to_timestamp_nanos` from DataFusion,
- restore the serde annotation to allow bincode deserialization of
  schemas in CubeStore,
- ignore tests that fail in our fork.

Author: ilya-biryukov

arrow-flight/Cargo.toml

@@ -41,7 +41,7 @@ futures = { version = "0.3", default-features = false, features = ["alloc"]}
 tonic-build = "0.4"
 # Pin specific version of the tonic-build dependencies to avoid auto-generated
 # (and checked in) arrow.flight.protocol.rs from changing
-proc-macro2 = "=1.0.27"
+proc-macro2 = "1.0.27"
 
 #[lib]
 #name = "flight"

arrow/src/compute/kernels/cast_utils.rs

@@ -16,7 +16,12 @@
 // under the License.
 
 use crate::error::{ArrowError, Result};
-use chrono::{prelude::*, LocalResult};
+use chrono::format::Fixed::{Nanosecond as FixedNanosecond, TimezoneOffsetColon};
+use chrono::format::Item::{Fixed, Literal, Numeric};
+use chrono::format::Numeric::Nanosecond;
+use chrono::format::Pad::Zero;
+use chrono::format::{Item, Parsed};
+use chrono::prelude::*;
 
 /// Accepts a string in RFC3339 / ISO8601 standard format and some
 /// variants and converts it to a nanosecond precision timestamp.
@@ -78,45 +83,92 @@ pub fn string_to_timestamp_nanos(s: &str) -> Result<i64> {
     // separating the date and time with a space ' ' rather than 'T' to be
     // (more) compatible with Apache Spark SQL
 
-    // timezone offset, using ' ' as a separator
-    // Example: 2020-09-08 13:42:29.190855-05:00
-    if let Ok(ts) = DateTime::parse_from_str(s, "%Y-%m-%d %H:%M:%S%.f%:z") {
-        return Ok(ts.timestamp_nanos());
+    // We parse the date and time prefix first to share work between all the different formats.
+    let mut rest = s;
+    let mut p;
+    let separator_is_space;
+    match try_parse_prefix(&mut rest) {
+        Some(ParsedPrefix {
+            result,
+            separator_is_space: s,
+        }) => {
+            p = result;
+            separator_is_space = s;
+        }
+        None => {
+            return Err(ArrowError::CastError(format!(
+                "Error parsing '{}' as timestamp",
+                s
+            )));
+        }
     }
 
-    // with an explicit Z, using ' ' as a separator
-    // Example: 2020-09-08 13:42:29Z
-    if let Ok(ts) = Utc.datetime_from_str(s, "%Y-%m-%d %H:%M:%S%.fZ") {
-        return Ok(ts.timestamp_nanos());
-    }
+    if separator_is_space {
+        // timezone offset, using ' ' as a separator
+        // Example: 2020-09-08 13:42:29.190855-05:00
+        // Full format string: "%Y-%m-%d %H:%M:%S%.f%:z".
+        const FORMAT1: [Item; 2] = [Fixed(FixedNanosecond), Fixed(TimezoneOffsetColon)];
+        if let Ok(ts) = chrono::format::parse(&mut p, rest, FORMAT1.iter())
+            .and_then(|()| p.to_datetime())
+        {
+            return Ok(ts.timestamp_nanos());
+        }
 
-    // Support timestamps without an explicit timezone offset, again
-    // to be compatible with what Apache Spark SQL does.
+        // with an explicit Z, using ' ' as a separator
+        // Example: 2020-09-08 13:42:29Z
+        // Full format string: "%Y-%m-%d %H:%M:%S%.fZ".
+        const FORMAT2: [Item; 2] = [Fixed(FixedNanosecond), Literal("Z")];
+        if let Ok(ts) = chrono::format::parse(&mut p, rest, FORMAT2.iter())
+            .and_then(|()| p.to_datetime_with_timezone(&Utc))
+        {
+            return Ok(ts.timestamp_nanos());
+        }
 
-    // without a timezone specifier as a local time, using T as a separator
-    // Example: 2020-09-08T13:42:29.190855
-    if let Ok(ts) = NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S.%f") {
-        return naive_datetime_to_timestamp(s, ts);
-    }
+        // without a timezone specifier as a local time, using ' ' as a separator
+        // Example: 2020-09-08 13:42:29.190855
+        const FORMAT5: [Item; 2] = [Literal("."), Numeric(Nanosecond, Zero)];
+        // Full format string: "%Y-%m-%d %H:%M:%S.%f".
+        if let Ok(ts) = chrono::format::parse(&mut p, rest, FORMAT5.iter())
+            .and_then(|()| p.to_naive_datetime_with_offset(0))
+        {
+            return naive_datetime_to_timestamp(s, ts);
+        }
 
-    // without a timezone specifier as a local time, using T as a
-    // separator, no fractional seconds
-    // Example: 2020-09-08T13:42:29
-    if let Ok(ts) = NaiveDateTime::parse_from_str(s, "%Y-%m-%dT%H:%M:%S") {
-        return naive_datetime_to_timestamp(s, ts);
+        // without a timezone specifier as a local time, using ' ' as a
+        // separator, no fractional seconds
+        // Example: 2020-09-08 13:42:29
+        // Full format string: "%Y-%m-%d %H:%M:%S".
+        if rest.is_empty() {
+            if let Ok(ts) = p.to_naive_datetime_with_offset(0) {
+                return naive_datetime_to_timestamp(s, ts);
+            }
+        }
     }
 
-    // without a timezone specifier as a local time, using ' ' as a separator
-    // Example: 2020-09-08 13:42:29.190855
-    if let Ok(ts) = NaiveDateTime::parse_from_str(s, "%Y-%m-%d %H:%M:%S.%f") {
-        return naive_datetime_to_timestamp(s, ts);
-    }
+    // Support timestamps without an explicit timezone offset, again
+    // to be compatible with what Apache Spark SQL does.
+    if !separator_is_space
+    /* i.e. separator == b'T' */
+    {
+        // without a timezone specifier as a local time, using T as a separator
+        // Example: 2020-09-08T13:42:29.190855
+        // Full format string: "%Y-%m-%dT%H:%M:%S.%f".
+        const FORMAT3: [Item; 2] = [Literal("."), Numeric(Nanosecond, Zero)];
+        if let Ok(ts) = chrono::format::parse(&mut p, rest, FORMAT3.iter())
+            .and_then(|()| p.to_naive_datetime_with_offset(0))
+        {
+            return naive_datetime_to_timestamp(s, ts);
+        }
 
-    // without a timezone specifier as a local time, using ' ' as a
-    // separator, no fractional seconds
-    // Example: 2020-09-08 13:42:29
-    if let Ok(ts) = NaiveDateTime::parse_from_str(s, "%Y-%m-%d %H:%M:%S") {
-        return naive_datetime_to_timestamp(s, ts);
+        // without a timezone specifier as a local time, using T as a
+        // separator, no fractional seconds
+        // Example: 2020-09-08T13:42:29
+        // Full format string: "%Y-%m-%dT%H:%M:%S".
+        if rest.is_empty() {
+            if let Ok(ts) = p.to_naive_datetime_with_offset(0) {
+                return naive_datetime_to_timestamp(s, ts);
+            }
+        }
     }
 
     // Note we don't pass along the error message from the underlying
@@ -130,33 +182,97 @@ pub fn string_to_timestamp_nanos(s: &str) -> Result<i64> {
     )))
 }
 
+/// Parses YYYY-MM-DD(T| )HH:MM:SS.
+fn try_parse_prefix(s: &mut &str) -> Option<ParsedPrefix> {
+    let mut p = Parsed::new();
+
+    let mut rest = s.as_bytes();
+    let year = try_parse_num(&mut rest)?;
+    try_consume(&mut rest, b'-')?;
+    let month = try_parse_num(&mut rest)?;
+    try_consume(&mut rest, b'-')?;
+    let day = try_parse_num(&mut rest)?;
+    if rest.is_empty() {
+        return None;
+    }
+
+    let separator_is_space;
+    match rest[0] {
+        b' ' => separator_is_space = true,
+        b'T' => separator_is_space = false,
+        _ => return None,
+    }
+
+    rest = &rest[1..];
+    let hour = try_parse_num(&mut rest)?;
+    try_consume(&mut rest, b':')?;
+    let minute = try_parse_num(&mut rest)?;
+    try_consume(&mut rest, b':')?;
+    let second = try_parse_num(&mut rest)?;
+
+    p.set_year(year).ok()?;
+    p.set_month(month).ok()?;
+    p.set_day(day).ok()?;
+    p.set_hour(hour).ok()?;
+    p.set_minute(minute).ok()?;
+    p.set_second(second).ok()?;
+
+    *s = unsafe { std::str::from_utf8_unchecked(rest) };
+    Some(ParsedPrefix {
+        result: p,
+        separator_is_space,
+    })
+}
+
+#[must_use]
+fn try_parse_num(s: &mut &[u8]) -> Option<i64> {
+    if s.is_empty() {
+        return None;
+    }
+
+    let mut i;
+    if s[0] == b'-' {
+        i = 1
+    } else {
+        i = 0;
+    }
+
+    while i < s.len() && b'0' <= s[i] && s[i] <= b'9' {
+        i += 1
+    }
+
+    let res = unsafe { std::str::from_utf8_unchecked(&s[0..i]) }
+        .parse()
+        .ok();
+    *s = &s[i..];
+    return res;
+}
+
+#[must_use]
+fn try_consume(s: &mut &[u8], c: u8) -> Option<()> {
+    if s.is_empty() || s[0] != c {
+        return None;
+    }
+    *s = &s[1..];
+    Some(())
+}
+
+struct ParsedPrefix {
+    result: Parsed,
+    separator_is_space: bool, // When false, the separator is 'T'.
+}
+
 /// Converts the naive datetime (which has no specific timezone) to a
 /// nanosecond epoch timestamp relative to UTC.
-fn naive_datetime_to_timestamp(s: &str, datetime: NaiveDateTime) -> Result<i64> {
-    let l = Local {};
-
-    match l.from_local_datetime(&datetime) {
-        LocalResult::None => Err(ArrowError::CastError(format!(
-            "Error parsing '{}' as timestamp: local time representation is invalid",
-            s
-        ))),
-        LocalResult::Single(local_datetime) => {
-            Ok(local_datetime.with_timezone(&Utc).timestamp_nanos())
-        }
-        // Ambiguous times can happen if the timestamp is exactly when
-        // a daylight savings time transition occurs, for example, and
-        // so the datetime could validly be said to be in two
-        // potential offsets. However, since we are about to convert
-        // to UTC anyways, we can pick one arbitrarily
-        LocalResult::Ambiguous(local_datetime, _) => {
-            Ok(local_datetime.with_timezone(&Utc).timestamp_nanos())
-        }
-    }
+fn naive_datetime_to_timestamp(_s: &str, datetime: NaiveDateTime) -> Result<i64> {
+    // CubeStore-specific: do not take timezones into account.
+    Ok(Utc.from_utc_datetime(&datetime).timestamp_nanos())
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use chrono::LocalResult;
 
     #[test]
     fn string_to_timestamp_timezone() -> Result<()> {
@@ -220,6 +336,7 @@ mod tests {
     }
 
     #[test]
+    #[ignore = "CubeStore always uses UTC"]
     #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function: mktime
     fn string_to_timestamp_no_timezone() -> Result<()> {
         // This test is designed to succeed in regardless of the local

arrow/src/datatypes/schema.rs

@@ -35,7 +35,7 @@ pub struct Schema {
     pub(crate) fields: Vec<Field>,
     /// A map of key-value pairs containing additional meta data.
     #[serde(skip_serializing_if = "HashMap::is_empty")]
-    #[serde(default)]
+    #[serde(skip_deserializing)]
     pub(crate) metadata: HashMap<String, String>,
 }
 
@@ -344,6 +344,7 @@ mod tests {
     use super::*;
 
     #[test]
+    #[ignore = "CubeStore ignores metadata during deserialization"]
     fn test_ser_de_metadata() {
         // ser/de with empty metadata
         let mut schema = Schema::new(vec![