Skip to content

Commit

Permalink
feat: time unit try from string (#45)
Browse files Browse the repository at this point in the history
# Rationale for this change

In DDL, a Timestamp column can be instantiated with the following
syntax:

```sql
TIMESTAMP(6)
```

where 6 represents millisecond precision. It appears that the gateway by
default supports only nanosecond precision on DDL, however if this
changes in the future, we will need a way to parse the time unit from a
DDL message in proofs-service. This PR contains a simple string parser
that allows for selection of a supported precision for timestamp
columns.

# What changes are included in this PR?

- [x] Implements TryFrom &str for TimeUnit
- [x] updates Display implementation for TImestamp column type to fix a
backwards print that is occuring:

```
column type TIMESTAMP(TIMEUNIT: UTC, TIMEZONE: NANOSECONDS (PRECISION: 9)) 
``` 

# Are these changes tested?

yes
  • Loading branch information
Dustin-Ray authored Jul 18, 2024
1 parent b52d19a commit 54584e5
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 7 deletions.
5 changes: 5 additions & 0 deletions crates/proof-of-sql-parser/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ pub enum PoSQLTimestampError {
/// Represents a catch-all for parsing errors not specifically covered by other variants.
#[error("Timestamp parsing error: {0}")]
ParsingError(String),

/// Represents a failure to parse a provided time unit precision value, PoSQL supports
/// Seconds, Milliseconds, Microseconds, and Nanoseconds
#[error("Timestamp parsing error: {0}")]
UnsupportedPrecision(String),
}

// This exists because TryFrom<DataType> for ColumnType error is String
Expand Down
40 changes: 38 additions & 2 deletions crates/proof-of-sql-parser/src/posql_time/unit.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::error::PoSQLTimestampError;
use arrow::datatypes::TimeUnit as ArrowTimeUnit;
use core::fmt;
use serde::{Deserialize, Serialize};
Expand All @@ -15,6 +16,19 @@ pub enum PoSQLTimeUnit {
Nanosecond,
}

impl TryFrom<&str> for PoSQLTimeUnit {
type Error = PoSQLTimestampError;
fn try_from(value: &str) -> Result<Self, PoSQLTimestampError> {
match value {
"0" => Ok(PoSQLTimeUnit::Second),
"3" => Ok(PoSQLTimeUnit::Millisecond),
"6" => Ok(PoSQLTimeUnit::Microsecond),
"9" => Ok(PoSQLTimeUnit::Nanosecond),
_ => Err(PoSQLTimestampError::UnsupportedPrecision(value.into())),
}
}
}

impl From<PoSQLTimeUnit> for ArrowTimeUnit {
fn from(unit: PoSQLTimeUnit) -> Self {
match unit {
Expand Down Expand Up @@ -53,10 +67,32 @@ impl fmt::Display for PoSQLTimeUnit {
#[cfg(test)]
#[allow(deprecated)]
mod time_unit_tests {

use crate::posql_time::{timestamp::PoSQLTimestamp, unit::PoSQLTimeUnit};
use super::*;
use crate::{error::PoSQLTimestampError, posql_time::timestamp::PoSQLTimestamp};
use chrono::{TimeZone, Utc};

#[test]
fn test_valid_precisions() {
assert_eq!(PoSQLTimeUnit::try_from("0"), Ok(PoSQLTimeUnit::Second));
assert_eq!(PoSQLTimeUnit::try_from("3"), Ok(PoSQLTimeUnit::Millisecond));
assert_eq!(PoSQLTimeUnit::try_from("6"), Ok(PoSQLTimeUnit::Microsecond));
assert_eq!(PoSQLTimeUnit::try_from("9"), Ok(PoSQLTimeUnit::Nanosecond));
}

#[test]
fn test_invalid_precision() {
let invalid_precisions = [
"1", "2", "4", "5", "7", "8", "10", "zero", "three", "cat", "-1", "-2",
]; // Testing all your various invalid inputs
for &value in invalid_precisions.iter() {
let result = PoSQLTimeUnit::try_from(value);
assert!(matches!(
result,
Err(PoSQLTimestampError::UnsupportedPrecision(_))
));
}
}

#[test]
fn test_rfc3339_timestamp_with_milliseconds() {
let input = "2023-06-26T12:34:56.123Z";
Expand Down
18 changes: 13 additions & 5 deletions crates/proof-of-sql/src/base/database/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,9 @@ impl std::fmt::Display for ColumnType {
}
ColumnType::VarChar => write!(f, "VARCHAR"),
ColumnType::Scalar => write!(f, "SCALAR"),
ColumnType::TimestampTZ(timeunit, timezone) => write!(
f,
"TIMESTAMP(TIMEUNIT: {:?}, TIMEZONE: {timeunit})",
timezone
),
ColumnType::TimestampTZ(timeunit, timezone) => {
write!(f, "TIMESTAMP(TIMEUNIT: {timeunit}, TIMEZONE: {timezone})")
}
}
}
}
Expand Down Expand Up @@ -507,6 +505,12 @@ mod tests {

#[test]
fn we_can_deserialize_columns_from_valid_strings() {
let expected_column_type =
ColumnType::TimestampTZ(PoSQLTimeUnit::Second, PoSQLTimeZone::Utc);
let deserialized: ColumnType =
serde_json::from_str(r#"{"TimestampTZ":["Second","Utc"]}"#).unwrap();
assert_eq!(deserialized, expected_column_type);

let expected_column_type = ColumnType::Boolean;
let deserialized: ColumnType = serde_json::from_str(r#""Boolean""#).unwrap();
assert_eq!(deserialized, expected_column_type);
Expand Down Expand Up @@ -658,6 +662,10 @@ mod tests {
let deserialized: Result<ColumnType, _> = serde_json::from_str(r#""DecImal75""#);
assert!(deserialized.is_err());

let deserialized: Result<ColumnType, _> =
serde_json::from_str(r#"{"TimestampTZ":["Utc","Second"]}"#);
assert!(deserialized.is_err());

let deserialized: Result<ColumnType, _> = serde_json::from_str(r#""Varchar""#);
assert!(deserialized.is_err());

Expand Down

0 comments on commit 54584e5

Please sign in to comment.