Return errors for invalid temporal types and values (#11015)

## Summary

Validates temporal dtypes and returns errors from fallible scalar and
import paths for invalid metadata or out-of-range values. Keeps the
current timestamp range limits. #10953 proposes a wider range, so the
two approaches can be reviewed together.

## Changes

Checks Date/Time storage and Timestamp metadata, uses fallible Span
setters, and covers Arrow, DataFusion, Python, and JNI boundaries.
Pinned formatting and five-crate Clippy pass, along with 6,412 Rust
tests (1 skipped), 73 Python tests, and 38 Java tests. Python type
checking, Ruff, and Spotless pass.

## API Changes

Adds `Timestamp::try_new_with_tz` and rejects Timestamp `Days` and
unresolved time zones at construction. Existing convenience constructors
retain a documented panic contract.

---------

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

Author: connortsui20

java/vortex-jni/src/test/java/dev/vortex/api/ExpressionTagParityTest.java

@@ -164,7 +164,7 @@ public void timestampAcceptsEveryUnitExceptDays() {
         RuntimeException exception =
                 assertThrows(RuntimeException.class, () -> Expression.literalTimestamp(0L, TimeUnit.DAYS, "UTC"));
         assertTrue(
-                exception.getMessage().contains("Timestamp does not support Days time unit"),
+                exception.getMessage().contains("Timestamp time unit must be finer than days, got Days"),
                 () -> "unexpected message: " + exception.getMessage());
     }
 

java/vortex-jni/src/test/java/dev/vortex/api/ExpressionTest.java

@@ -21,6 +21,25 @@ public static void loadLibrary() {
         NativeLoader.loadJni();
     }
 
+    @Test
+    public void timestampLiteralsRejectInvalidMetadata() {
+        assertThrows(
+                RuntimeException.class,
+                () -> Expression.literalTimestamp(0L, Expression.TimeUnit.SECONDS, "Not/A/Timezone"));
+        assertThrows(
+                RuntimeException.class,
+                () -> Expression.nullLiteralTimestamp(Expression.TimeUnit.SECONDS, "Not/A/Timezone"));
+        assertThrows(RuntimeException.class, () -> Expression.nullLiteralTimestamp(Expression.TimeUnit.DAYS, null));
+    }
+
+    @Test
+    public void timestampLiteralsRejectOutOfRangeValues() {
+        assertThrows(
+                RuntimeException.class,
+                () -> Expression.literalTimestamp(Long.MAX_VALUE, Expression.TimeUnit.SECONDS, null));
+        assertNotNull(Expression.literalTimestamp(0L, Expression.TimeUnit.SECONDS, "UTC"));
+    }
+
     @Test
     public void rowIdxBuildsAndComposes() {
         assertNotNull(Expression.rowIdx());

vortex-array/src/arrays/datetime/mod.rs

@@ -100,14 +100,12 @@ impl TemporalData {
         }
     }
 
-    /// Create a new `TemporalData` holding Arrow spec compliant Timestamp data, with an
-    /// optional timezone.
+    /// Creates a timestamp array with the given unit and optional time zone.
     ///
     /// # Panics
     ///
-    /// If `array` does not hold Primitive i64 data, the function will panic.
-    ///
-    /// If the time_unit is days, the function will panic.
+    /// Panics if `array` does not have primitive i64 storage, or if
+    /// [`Timestamp::try_new_with_tz`] rejects the unit or time zone.
     pub fn new_timestamp(
         array: ArrayRef,
         time_unit: TimeUnit,

vortex-array/src/dtype/dtype_impl.rs

@@ -606,7 +606,8 @@ mod tests {
             Timestamp::new_with_tz(TimeUnit::Seconds, Some("UTC".into()), Nullable).erased(),
         );
         let t2 = DType::Extension(
-            Timestamp::new_with_tz(TimeUnit::Seconds, Some("ET".into()), Nullable).erased(),
+            Timestamp::new_with_tz(TimeUnit::Seconds, Some("America/New_York".into()), Nullable)
+                .erased(),
         );
         assert!(!t1.eq_ignore_nullability(&t2));
     }

vortex-array/src/dtype/serde/proto.rs

@@ -456,7 +456,7 @@ mod tests {
     #[test]
     fn test_extension_round_trip() {
         let ext_dtype =
-            DType::Extension(Timestamp::new(TimeUnit::Days, Nullability::Nullable).erased());
+            DType::Extension(Timestamp::new(TimeUnit::Seconds, Nullability::Nullable).erased());
         let converted = round_trip_dtype(&ext_dtype);
         assert_eq!(ext_dtype, converted);
     }

vortex-array/src/extension/datetime/date.rs

@@ -100,12 +100,13 @@ impl ExtVTable for Date {
         let ptype = date_ptype(metadata)
             .ok_or_else(|| vortex_err!("Date type does not support time unit {}", metadata))?;
 
-        vortex_ensure_eq!(
-            ext_dtype.storage_dtype().as_ptype(),
-            ptype,
-            "Date storage dtype for {} has the wrong ptype",
-            metadata
-        );
+        let DType::Primitive(storage_ptype, _) = ext_dtype.storage_dtype() else {
+            vortex_bail!(
+                "Date storage dtype must be primitive, got {}",
+                ext_dtype.storage_dtype()
+            );
+        };
+        vortex_ensure_eq!(*storage_ptype, ptype);
 
         Ok(())
     }
@@ -131,12 +132,18 @@ mod tests {
 
     use crate::dtype::DType;
     use crate::dtype::Nullability::Nullable;
+    use crate::dtype::extension::ExtDType;
     use crate::extension::datetime::Date;
     use crate::extension::datetime::TimeUnit;
     use crate::scalar::PValue;
     use crate::scalar::Scalar;
     use crate::scalar::ScalarValue;
 
+    #[test]
+    fn test_reject_non_primitive_storage() {
+        assert!(ExtDType::<Date>::try_new(TimeUnit::Days, DType::Utf8(Nullable),).is_err());
+    }
+
     #[test]
     fn validate_date_scalar() -> VortexResult<()> {
         let days_dtype = DType::Extension(Date::new(TimeUnit::Days, Nullable).erased());

vortex-array/src/extension/datetime/time.rs

@@ -101,12 +101,13 @@ impl ExtVTable for Time {
         let ptype = time_ptype(metadata)
             .ok_or_else(|| vortex_err!("Time type does not support time unit {}", metadata))?;
 
-        vortex_ensure_eq!(
-            ext_dtype.storage_dtype().as_ptype(),
-            ptype,
-            "Time storage dtype for {} has the wrong ptype",
-            metadata
-        );
+        let DType::Primitive(storage_ptype, _) = ext_dtype.storage_dtype() else {
+            vortex_bail!(
+                "Time storage dtype must be primitive, got {}",
+                ext_dtype.storage_dtype()
+            );
+        };
+        vortex_ensure_eq!(*storage_ptype, ptype);
 
         Ok(())
     }
@@ -121,19 +122,19 @@ impl ExtVTable for Time {
             TimeUnit::Seconds => {
                 let v = i32::try_from(length_of_time)
                     .map_err(|e| vortex_err!("Time seconds value out of i32 range: {e}"))?;
-                (Span::new().seconds(v), TimeValue::Seconds(v))
+                (Span::new().try_seconds(v)?, TimeValue::Seconds(v))
             }
             TimeUnit::Milliseconds => {
                 let v = i32::try_from(length_of_time)
                     .map_err(|e| vortex_err!("Time milliseconds value out of i32 range: {e}"))?;
-                (Span::new().milliseconds(v), TimeValue::Milliseconds(v))
+                (Span::new().try_milliseconds(v)?, TimeValue::Milliseconds(v))
             }
             TimeUnit::Microseconds => (
-                Span::new().microseconds(length_of_time),
+                Span::new().try_microseconds(length_of_time)?,
                 TimeValue::Microseconds(length_of_time),
             ),
             TimeUnit::Nanoseconds => (
-                Span::new().nanoseconds(length_of_time),
+                Span::new().try_nanoseconds(length_of_time)?,
                 TimeValue::Nanoseconds(length_of_time),
             ),
             d @ TimeUnit::Days => {
@@ -152,16 +153,31 @@ impl ExtVTable for Time {
 
 #[cfg(test)]
 mod tests {
+    use rstest::rstest;
     use vortex_error::VortexResult;
 
     use crate::dtype::DType;
     use crate::dtype::Nullability::Nullable;
+    use crate::dtype::extension::ExtDType;
     use crate::extension::datetime::Time;
     use crate::extension::datetime::TimeUnit;
     use crate::scalar::PValue;
     use crate::scalar::Scalar;
     use crate::scalar::ScalarValue;
 
+    #[rstest]
+    #[case::microseconds(TimeUnit::Microseconds, i64::MAX)]
+    #[case::nanoseconds(TimeUnit::Nanoseconds, i64::MIN)]
+    fn test_reject_time_span_out_of_range(#[case] unit: TimeUnit, #[case] value: i64) {
+        let dtype = DType::Extension(Time::new(unit, Nullable).erased());
+        assert!(Scalar::try_new(dtype, Some(ScalarValue::from(value))).is_err());
+    }
+
+    #[test]
+    fn test_reject_non_primitive_storage() {
+        assert!(ExtDType::<Time>::try_new(TimeUnit::Seconds, DType::Utf8(Nullable),).is_err());
+    }
+
     #[test]
     fn validate_time_scalar() -> VortexResult<()> {
         // 3661 seconds = 1 hour, 1 minute, 1 second.

vortex-array/src/extension/datetime/timestamp.rs

@@ -31,31 +31,56 @@ use crate::scalar::ScalarValue;
 pub struct Timestamp;
 
 impl Timestamp {
-    /// Creates a new Timestamp extension =dtype with the given time unit and nullability.
+    /// Creates a timestamp dtype with the given time unit and nullability.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `time_unit` is [`TimeUnit::Days`].
     pub fn new(time_unit: TimeUnit, nullability: Nullability) -> ExtDType<Self> {
         Self::new_with_tz(time_unit, None, nullability)
     }
 
-    /// Creates a new Timestamp extension dtype with the given time unit, timezone, and nullability.
-    pub fn new_with_tz(
+    /// Creates a timestamp dtype with the given time unit, time zone, and nullability.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error for [`TimeUnit::Days`] or an unresolved time zone. IANA names and fixed
+    /// UTC offsets are accepted.
+    pub fn try_new_with_tz(
         time_unit: TimeUnit,
         timezone: Option<Arc<str>>,
         nullability: Nullability,
-    ) -> ExtDType<Self> {
+    ) -> VortexResult<ExtDType<Self>> {
         ExtDType::try_new(
             TimestampOptions {
                 unit: time_unit,
                 tz: timezone,
             },
             DType::Primitive(PType::I64, nullability),
         )
-        .vortex_expect("failed to create timestamp dtype")
     }
 
-    /// Creates a new `Timestamp` extension dtype with the given options and nullability.
+    /// Creates a timestamp dtype with the given time unit, time zone, and nullability.
+    ///
+    /// # Panics
+    ///
+    /// Panics if [`Self::try_new_with_tz`] rejects the unit or time zone.
+    pub fn new_with_tz(
+        time_unit: TimeUnit,
+        timezone: Option<Arc<str>>,
+        nullability: Nullability,
+    ) -> ExtDType<Self> {
+        Self::try_new_with_tz(time_unit, timezone, nullability)
+            .vortex_expect("timestamp unit and time zone must be valid")
+    }
+
+    /// Creates a timestamp dtype with the given options and nullability.
+    ///
+    /// # Panics
+    ///
+    /// Panics if [`Self::try_new_with_tz`] rejects the unit or time zone.
     pub fn new_with_options(options: TimestampOptions, nullability: Nullability) -> ExtDType<Self> {
-        ExtDType::try_new(options, DType::Primitive(PType::I64, nullability))
-            .vortex_expect("failed to create timestamp dtype")
+        Self::new_with_tz(options.unit, options.tz, nullability)
     }
 }
 
@@ -200,6 +225,13 @@ impl ExtVTable for Timestamp {
             matches!(ext_dtype.storage_dtype(), DType::Primitive(PType::I64, _)),
             "Timestamp storage dtype must be i64"
         );
+        vortex_ensure!(
+            ext_dtype.metadata().unit != TimeUnit::Days,
+            "Timestamp time unit must be finer than days, got {:?}",
+            ext_dtype.metadata().unit
+        );
+        resolve_time_zone(ext_dtype.metadata().tz.as_deref())?;
+
         Ok(())
     }
 
@@ -213,19 +245,19 @@ impl ExtVTable for Timestamp {
 
         let (span, value) = match metadata.unit {
             TimeUnit::Nanoseconds => (
-                Span::new().nanoseconds(ts_value),
+                Span::new().try_nanoseconds(ts_value)?,
                 TimestampValue::Nanoseconds(ts_value, tz),
             ),
             TimeUnit::Microseconds => (
-                Span::new().microseconds(ts_value),
+                Span::new().try_microseconds(ts_value)?,
                 TimestampValue::Microseconds(ts_value, tz),
             ),
             TimeUnit::Milliseconds => (
-                Span::new().milliseconds(ts_value),
+                Span::new().try_milliseconds(ts_value)?,
                 TimestampValue::Milliseconds(ts_value, tz),
             ),
             TimeUnit::Seconds => (
-                Span::new().seconds(ts_value),
+                Span::new().try_seconds(ts_value)?,
                 TimestampValue::Seconds(ts_value, tz),
             ),
             TimeUnit::Days => {
@@ -238,11 +270,6 @@ impl ExtVTable for Timestamp {
             .checked_add(span)
             .map_err(|e| vortex_err!("Invalid timestamp scalar: {}", e))?;
 
-        if let Some(tz) = tz {
-            resolve_time_zone(Some(tz))
-                .map_err(|e| e.with_context("Invalid timezone for timestamp scalar"))?;
-        }
-
         Ok(value)
     }
 }
@@ -289,16 +316,29 @@ mod tests {
     #[cfg_attr(miri, ignore)]
     #[test]
     fn reject_timestamp_with_invalid_timezone() {
-        let dtype = DType::Extension(
-            Timestamp::new_with_tz(
+        assert!(
+            Timestamp::try_new_with_tz(
                 TimeUnit::Seconds,
                 Some(Arc::from("Not/A/Timezone")),
                 Nullable,
             )
-            .erased(),
+            .is_err()
         );
-        let result = Scalar::try_new(dtype, Some(ScalarValue::Primitive(PValue::I64(0))));
-        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_reject_timestamp_days() {
+        assert!(Timestamp::try_new_with_tz(TimeUnit::Days, None, Nullable).is_err());
+    }
+
+    #[rstest]
+    #[case::seconds(TimeUnit::Seconds, i64::MAX)]
+    #[case::milliseconds(TimeUnit::Milliseconds, i64::MAX)]
+    #[case::microseconds(TimeUnit::Microseconds, i64::MAX)]
+    #[case::nanoseconds(TimeUnit::Nanoseconds, i64::MIN)]
+    fn test_reject_timestamp_span_out_of_range(#[case] unit: TimeUnit, #[case] value: i64) {
+        let dtype = DType::Extension(Timestamp::new(unit, Nullable).erased());
+        assert!(Scalar::try_new(dtype, Some(ScalarValue::from(value))).is_err());
     }
 
     #[cfg_attr(miri, ignore)]

vortex-array/src/scalar/tests/nullability.rs

@@ -26,15 +26,15 @@ mod tests {
             DType::Bool(Nullability::Nullable),
             DType::Primitive(PType::I32, Nullability::Nullable),
             DType::Extension(Date::new(TimeUnit::Days, Nullability::Nullable).erased()),
-            DType::Extension(Timestamp::new(TimeUnit::Days, Nullability::Nullable).erased()),
+            DType::Extension(Timestamp::new(TimeUnit::Seconds, Nullability::Nullable).erased()),
         )]
         source_dtype: DType,
         #[values(
             DType::Null,
             DType::Bool(Nullability::Nullable),
             DType::Primitive(PType::I32, Nullability::Nullable),
             DType::Extension(Date::new(TimeUnit::Days, Nullability::Nullable).erased()),
-            DType::Extension(Timestamp::new(TimeUnit::Days, Nullability::Nullable).erased()),
+            DType::Extension(Timestamp::new(TimeUnit::Seconds, Nullability::Nullable).erased()),
         )]
         target_dtype: DType,
     ) {