Skip to content

Commit

Permalink
refactor: reorganize timestamp (#30)
Browse files Browse the repository at this point in the history
# Rationale for this change

The timestamp module is large enough now to warrant its own submodule.
It will grow in size with the addition of the intermediate timestamp
conversions.

# What changes are included in this PR?

Moves various components of timestamp into appropriate modules.

# Are these changes tested?

yes
  • Loading branch information
Dustin-Ray authored Jun 27, 2024
1 parent f0b2003 commit 7152021
Show file tree
Hide file tree
Showing 14 changed files with 162 additions and 149 deletions.
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/base/commitment/column_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ mod tests {
database::OwnedColumn,
math::decimal::Precision,
scalar::Curve25519Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
use itertools::Itertools;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ mod tests {
database::OwnedColumn,
math::decimal::Precision,
scalar::Curve25519Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};

#[test]
Expand Down
7 changes: 5 additions & 2 deletions crates/proof-of-sql/src/base/commitment/committable_column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::base::{
math::decimal::Precision,
ref_into::RefInto,
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
#[cfg(feature = "blitzar")]
use blitzar::sequence::Sequence;
Expand Down Expand Up @@ -194,7 +194,10 @@ impl<'a, 'b> From<&'a CommittableColumn<'b>> for Sequence<'a> {
#[cfg(all(test, feature = "blitzar"))]
mod tests {
use super::*;
use crate::{base::scalar::Curve25519Scalar, proof_primitive::dory::DoryScalar};
use crate::{
base::{scalar::Curve25519Scalar, time::timezone::PoSQLTimeZone},
proof_primitive::dory::DoryScalar,
};
use blitzar::compute::compute_curve25519_commitments;
use curve25519_dalek::ristretto::CompressedRistretto;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
database::Column,
math::decimal::Precision,
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
},
sql::parse::ConversionError,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/base/database/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::{LiteralValue, TableRef};
use crate::base::{
math::decimal::{scale_scalar, Precision},
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
use arrow::datatypes::{DataType, Field, TimeUnit as ArrowTimeUnit};
use bumpalo::Bump;
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/base/database/literal_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::base::{
database::ColumnType,
math::decimal::Precision,
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
use serde::{Deserialize, Serialize};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::base::{
},
math::decimal::Precision,
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
use arrow::{
array::{
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/base/database/owned_column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::ColumnType;
use crate::base::{
math::decimal::Precision,
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
#[derive(Debug, PartialEq, Clone, Eq)]
#[non_exhaustive]
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/base/database/owned_table_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
base::{
database::{owned_table_utility::*, OwnedColumn, OwnedTable, OwnedTableError},
scalar::Curve25519Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
},
proof_primitive::dory::DoryScalar,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::{
use crate::base::{
database::owned_table_utility::*,
scalar::{compute_commitment_for_testing, Curve25519Scalar},
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
use blitzar::proof::InnerProductProof;

Expand Down
5 changes: 3 additions & 2 deletions crates/proof-of-sql/src/base/database/owned_table_utility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use super::{OwnedColumn, OwnedTable};
use crate::base::{
scalar::Scalar,
time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone},
time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone},
};
use core::ops::Deref;
use proof_of_sql_parser::Identifier;
Expand Down Expand Up @@ -212,7 +212,8 @@ pub fn decimal75<S: Scalar>(
/// ```
/// use proof_of_sql::base::{database::owned_table_utility::*,
/// scalar::Curve25519Scalar,
/// time::timestamp::{PoSQLTimeUnit, PoSQLTimeZone}};
/// time::{timestamp::PoSQLTimeUnit, timezone::PoSQLTimeZone}
/// };
/// use chrono_tz::Europe::London;
///
/// let result = owned_table::<Curve25519Scalar>([
Expand Down
4 changes: 3 additions & 1 deletion crates/proof-of-sql/src/base/time/mod.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
/// Stores all functionality relelvant to timestamps
/// Stores all functionality relelvant to timestamps and time units
pub mod timestamp;
/// Stores functionality relevant to timezones
pub mod timezone;
135 changes: 0 additions & 135 deletions crates/proof-of-sql/src/base/time/timestamp.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use crate::base::database::{ArrowArrayToColumnConversionError, OwnedArrowConversionError};
use arrow::datatypes::TimeUnit as ArrowTimeUnit;
use chrono_tz::Tz;
use core::fmt;
use serde::{Deserialize, Serialize};
use std::{str::FromStr, sync::Arc};

/// A wrapper around i64 to mitigate conflicting From<i64>
/// implementations
Expand All @@ -15,66 +12,6 @@ pub struct Time {
pub unit: PoSQLTimeUnit,
}

/// A typed TimeZone for a [`TimeStamp`]. It is optionally
/// used to define a timezone other than UTC for a new TimeStamp.
/// It exists as a wrapper around chrono-tz because chrono-tz does
/// not implement uniform bit distribution
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Serialize, Deserialize)]
pub struct PoSQLTimeZone(Tz);

impl PoSQLTimeZone {
/// Convenience constant for the UTC timezone
pub const UTC: PoSQLTimeZone = PoSQLTimeZone(Tz::UTC);
}

impl PoSQLTimeZone {
/// Create a new ProofsTimeZone from a chrono TimeZone
pub fn new(tz: Tz) -> Self {
PoSQLTimeZone(tz)
}
}

impl From<&PoSQLTimeZone> for Arc<str> {
fn from(timezone: &PoSQLTimeZone) -> Self {
Arc::from(timezone.0.name())
}
}

impl From<Tz> for PoSQLTimeZone {
fn from(tz: Tz) -> Self {
PoSQLTimeZone(tz)
}
}

impl fmt::Display for PoSQLTimeZone {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}

impl TryFrom<Option<Arc<str>>> for PoSQLTimeZone {
type Error = &'static str;

fn try_from(value: Option<Arc<str>>) -> Result<Self, Self::Error> {
match value {
Some(arc_str) => Tz::from_str(&arc_str)
.map(PoSQLTimeZone)
.map_err(|_| "Invalid timezone string"),
None => Ok(PoSQLTimeZone(Tz::UTC)), // Default to UTC
}
}
}

impl TryFrom<&str> for PoSQLTimeZone {
type Error = &'static str;

fn try_from(value: &str) -> Result<Self, Self::Error> {
Tz::from_str(value)
.map(PoSQLTimeZone)
.map_err(|_| "Invalid timezone string")
}
}

/// Specifies different units of time measurement relative to the Unix epoch. It is essentially
/// a wrapper over [arrow::datatypes::TimeUnit] so that we can derive Copy and implement custom traits
/// such as bit distribution and Hash.
Expand Down Expand Up @@ -123,81 +60,9 @@ impl From<ArrowTimeUnit> for PoSQLTimeUnit {
}
}

impl From<&'static str> for OwnedArrowConversionError {
fn from(error: &'static str) -> Self {
OwnedArrowConversionError::InvalidTimezone(error.to_string())
}
}

impl From<&'static str> for ArrowArrayToColumnConversionError {
fn from(error: &'static str) -> Self {
ArrowArrayToColumnConversionError::TimezoneConversionError(error.to_string())
}
}

#[cfg(test)]
mod tests {
use super::*;
use chrono_tz::Tz;

#[test]
fn valid_timezones_convert_correctly() {
let valid_timezones = ["Europe/London", "America/New_York", "Asia/Tokyo", "UTC"];

for tz_str in &valid_timezones {
let arc_tz = Arc::new(tz_str.to_string());
// Convert Arc<String> to Arc<str> by dereferencing to &str then creating a new Arc
let arc_tz_str: Arc<str> = Arc::from(&**arc_tz);
let timezone = PoSQLTimeZone::try_from(Some(arc_tz_str));
assert!(timezone.is_ok(), "Timezone should be valid: {}", tz_str);
assert_eq!(
timezone.unwrap().0,
Tz::from_str(tz_str).unwrap(),
"Timezone mismatch for {}",
tz_str
);
}
}

#[test]
fn test_edge_timezone_strings() {
let edge_timezones = ["Etc/GMT+12", "Etc/GMT-14", "America/Argentina/Ushuaia"];
for tz_str in &edge_timezones {
let arc_tz = Arc::from(*tz_str);
let result = PoSQLTimeZone::try_from(Some(arc_tz));
assert!(result.is_ok(), "Edge timezone should be valid: {}", tz_str);
assert_eq!(
result.unwrap().0,
Tz::from_str(tz_str).unwrap(),
"Mismatch for edge timezone {}",
tz_str
);
}
}

#[test]
fn test_empty_timezone_string() {
let empty_tz = Arc::from("");
let result = PoSQLTimeZone::try_from(Some(empty_tz));
assert!(result.is_err(), "Empty timezone string should fail");
}

#[test]
fn test_unicode_timezone_strings() {
let unicode_tz = Arc::from("Europe/Paris\u{00A0}"); // Non-breaking space character
let result = PoSQLTimeZone::try_from(Some(unicode_tz));
assert!(
result.is_err(),
"Unicode characters should not be valid in timezone strings"
);
}

#[test]
fn test_null_option() {
let result = PoSQLTimeZone::try_from(None);
assert!(result.is_ok(), "None should convert without error");
assert_eq!(result.unwrap().0, Tz::UTC, "None should default to UTC");
}

#[test]
fn we_can_convert_from_arrow_time_units() {
Expand Down
Loading

0 comments on commit 7152021

Please sign in to comment.