diff --git a/vortex-array/src/array/datetime/mod.rs b/vortex-array/src/array/datetime/mod.rs index 9f8a2cab1c..686bccd043 100644 --- a/vortex-array/src/array/datetime/mod.rs +++ b/vortex-array/src/array/datetime/mod.rs @@ -1,6 +1,8 @@ #[cfg(test)] mod test; +use std::sync::Arc; + use vortex_datetime_dtype::{TemporalMetadata, TimeUnit, DATE_ID, TIMESTAMP_ID, TIME_ID}; use vortex_dtype::{DType, ExtDType}; use vortex_error::{vortex_panic, VortexError}; @@ -68,26 +70,22 @@ impl TemporalArray { /// /// If any other time unit is provided, it panics. pub fn new_date(array: Array, time_unit: TimeUnit) -> Self { - let ext_dtype = match time_unit { + match time_unit { TimeUnit::D => { assert_width!(i32, array); - - ExtDType::new( - DATE_ID.clone(), - Some(TemporalMetadata::Date(time_unit).into()), - ) } TimeUnit::Ms => { assert_width!(i64, array); - - ExtDType::new( - DATE_ID.clone(), - Some(TemporalMetadata::Date(time_unit).into()), - ) } _ => vortex_panic!("invalid TimeUnit {time_unit} for vortex.date"), }; + let ext_dtype = ExtDType::new( + DATE_ID.clone(), + Arc::new(array.dtype().clone()), + Some(TemporalMetadata::Date(time_unit).into()), + ); + Self { ext: ExtensionArray::new(ext_dtype, array), temporal_metadata: TemporalMetadata::Date(time_unit), @@ -123,7 +121,11 @@ impl TemporalArray { let temporal_metadata = TemporalMetadata::Time(time_unit); Self { ext: ExtensionArray::new( - ExtDType::new(TIME_ID.clone(), Some(temporal_metadata.clone().into())), + ExtDType::new( + TIME_ID.clone(), + Arc::new(array.dtype().clone()), + Some(temporal_metadata.clone().into()), + ), array, ), temporal_metadata, @@ -145,7 +147,11 @@ impl TemporalArray { Self { ext: ExtensionArray::new( - ExtDType::new(TIMESTAMP_ID.clone(), Some(temporal_metadata.clone().into())), + ExtDType::new( + TIMESTAMP_ID.clone(), + Arc::new(array.dtype().clone()), + Some(temporal_metadata.clone().into()), + ), array, ), temporal_metadata, diff --git a/vortex-datafusion/src/datatype.rs b/vortex-datafusion/src/datatype.rs index 6848850056..fc2f61aea4 100644 --- a/vortex-datafusion/src/datatype.rs +++ b/vortex-datafusion/src/datatype.rs @@ -95,6 +95,8 @@ pub(crate) fn infer_data_type(dtype: &DType) -> DataType { if is_temporal_ext_type(ext_dtype.id()) { make_arrow_temporal_dtype(ext_dtype) } else { + // TODO(aduffy): allow extension type authors to plugin their own to/from Arrow + // conversions. vortex_panic!("Unsupported extension type \"{}\"", ext_dtype.id()) } } @@ -167,7 +169,11 @@ mod test { #[should_panic] fn test_dtype_conversion_panics() { let _ = infer_data_type(&DType::Extension( - ExtDType::new(ExtID::from("my-fake-ext-dtype"), None), + ExtDType::new( + ExtID::from("my-fake-ext-dtype"), + Arc::new(PType::I32.into()), + None, + ), Nullability::NonNullable, )); } diff --git a/vortex-datetime-dtype/src/arrow.rs b/vortex-datetime-dtype/src/arrow.rs index 7af9d2c7fa..f4ad150af4 100644 --- a/vortex-datetime-dtype/src/arrow.rs +++ b/vortex-datetime-dtype/src/arrow.rs @@ -1,7 +1,9 @@ #![cfg(feature = "arrow")] +use std::sync::Arc; + use arrow_schema::{DataType, TimeUnit as ArrowTimeUnit}; -use vortex_dtype::ExtDType; +use vortex_dtype::{ExtDType, PType}; use vortex_error::{vortex_bail, vortex_panic, VortexError, VortexExpect as _, VortexResult}; use crate::temporal::{TemporalMetadata, DATE_ID, TIMESTAMP_ID, TIME_ID}; @@ -17,9 +19,10 @@ pub fn make_temporal_ext_dtype(data_type: &DataType) -> ExtDType { DataType::Timestamp(time_unit, time_zone) => { let time_unit = TimeUnit::from(time_unit); let tz = time_zone.clone().map(|s| s.to_string()); - + // PType is inferred for arrow based on the time units. ExtDType::new( TIMESTAMP_ID.clone(), + Arc::new(PType::I64.into()), Some(TemporalMetadata::Timestamp(time_unit, tz).into()), ) } @@ -27,6 +30,7 @@ pub fn make_temporal_ext_dtype(data_type: &DataType) -> ExtDType { let time_unit = TimeUnit::from(time_unit); ExtDType::new( TIME_ID.clone(), + Arc::new(PType::I32.into()), Some(TemporalMetadata::Time(time_unit).into()), ) } @@ -34,15 +38,18 @@ pub fn make_temporal_ext_dtype(data_type: &DataType) -> ExtDType { let time_unit = TimeUnit::from(time_unit); ExtDType::new( TIME_ID.clone(), + Arc::new(PType::I64.into()), Some(TemporalMetadata::Time(time_unit).into()), ) } DataType::Date32 => ExtDType::new( DATE_ID.clone(), + Arc::new(PType::I32.into()), Some(TemporalMetadata::Date(TimeUnit::D).into()), ), DataType::Date64 => ExtDType::new( DATE_ID.clone(), + Arc::new(PType::I64.into()), Some(TemporalMetadata::Date(TimeUnit::Ms).into()), ), _ => unimplemented!("{data_type} conversion"), diff --git a/vortex-datetime-dtype/src/temporal.rs b/vortex-datetime-dtype/src/temporal.rs index e824c1952c..86ec9e4561 100644 --- a/vortex-datetime-dtype/src/temporal.rs +++ b/vortex-datetime-dtype/src/temporal.rs @@ -188,7 +188,9 @@ impl From for ExtMetadata { #[cfg(test)] mod tests { - use vortex_dtype::{ExtDType, ExtMetadata}; + use std::sync::Arc; + + use vortex_dtype::{ExtDType, ExtMetadata, PType}; use crate::{TemporalMetadata, TimeUnit, TIMESTAMP_ID}; @@ -207,8 +209,12 @@ mod tests { .as_slice() ); - let temporal_metadata = - TemporalMetadata::try_from(&ExtDType::new(TIMESTAMP_ID.clone(), Some(meta))).unwrap(); + let temporal_metadata = TemporalMetadata::try_from(&ExtDType::new( + TIMESTAMP_ID.clone(), + Arc::new(PType::I64.into()), + Some(meta), + )) + .unwrap(); assert_eq!( temporal_metadata, diff --git a/vortex-dtype/src/dtype.rs b/vortex-dtype/src/dtype.rs index 0e6e90f3c2..eda0b4ae89 100644 --- a/vortex-dtype/src/dtype.rs +++ b/vortex-dtype/src/dtype.rs @@ -135,8 +135,9 @@ impl Display for DType { List(c, n) => write!(f, "list({}){}", c, n), Extension(ext, n) => write!( f, - "ext({}{}){}", + "ext({}, {}{}){}", ext.id(), + ext.scalars_dtype(), ext.metadata() .map(|m| format!(", {:?}", m)) .unwrap_or_else(|| "".to_string()), diff --git a/vortex-dtype/src/extension.rs b/vortex-dtype/src/extension.rs index 18daec5505..b67a260f39 100644 --- a/vortex-dtype/src/extension.rs +++ b/vortex-dtype/src/extension.rs @@ -1,6 +1,8 @@ use std::fmt::{Display, Formatter}; use std::sync::Arc; +use crate::DType; + #[derive(Debug, Clone, PartialEq, Eq, Ord, PartialOrd, Hash)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] pub struct ExtID(Arc); @@ -55,12 +57,47 @@ impl From<&[u8]> for ExtMetadata { #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct ExtDType { id: ExtID, + scalars_dtype: Arc, metadata: Option, } impl ExtDType { - pub fn new(id: ExtID, metadata: Option) -> Self { - Self { id, metadata } + /// Creates a new `ExtDType`. + /// + /// Extension data types in Vortex allows library users to express additional semantic meaning + /// on top of a set of scalar values. Metadata can optionally be provided for the extension type + /// to allow for parameterized types. + /// + /// A simple example would be if one wanted to create a `vortex.temperature` extension type. The + /// canonical encoding for such values would be `f64`, and the metadata can contain an optional + /// temperature unit, allowing downstream users to be sure they properly account for Celsius + /// and Fahrenheit conversions. + /// + /// ``` + /// use std::sync::Arc; + /// use vortex_dtype::{DType, ExtDType, ExtID, ExtMetadata, Nullability, PType}; + /// + /// #[repr(u8)] + /// enum TemperatureUnit { + /// C = 0u8, + /// F = 1u8, + /// } + /// + /// // Make a new extension type that encodes the unit for a set of nullable `f64`. + /// pub fn create_temperature_type(unit: TemperatureUnit) -> ExtDType { + /// ExtDType::new( + /// ExtID::new("vortex.temperature".into()), + /// Arc::new(DType::Primitive(PType::F64, Nullability::Nullable)), + /// Some(ExtMetadata::new([unit as u8].into())) + /// ) + /// } + /// ``` + pub fn new(id: ExtID, scalars_dtype: Arc, metadata: Option) -> Self { + Self { + id, + scalars_dtype, + metadata, + } } #[inline] @@ -68,6 +105,11 @@ impl ExtDType { &self.id } + #[inline] + pub fn scalars_dtype(&self) -> &DType { + self.scalars_dtype.as_ref() + } + #[inline] pub fn metadata(&self) -> Option<&ExtMetadata> { self.metadata.as_ref() diff --git a/vortex-dtype/src/serde/flatbuffers/mod.rs b/vortex-dtype/src/serde/flatbuffers/mod.rs index 5a2b46d101..2264cfd5f2 100644 --- a/vortex-dtype/src/serde/flatbuffers/mod.rs +++ b/vortex-dtype/src/serde/flatbuffers/mod.rs @@ -87,7 +87,19 @@ impl TryFrom> for DType { })?); let metadata = fb_ext.metadata().map(|m| ExtMetadata::from(m.bytes())); Ok(Self::Extension( - ExtDType::new(id, metadata), + ExtDType::new( + id, + Arc::new( + DType::try_from(fb_ext.scalars_dtype().ok_or_else(|| { + vortex_err!( + InvalidSerde: "scalars_dtype must be present on DType fbs message") + })?) + .map_err(|e| { + vortex_err!("failed to create DType from fbs message: {e}") + })?, + ), + metadata, + ), fb_ext.nullable().into(), )) } @@ -173,11 +185,13 @@ impl WriteFlatBuffer for DType { } Self::Extension(ext, n) => { let id = Some(fbb.create_string(ext.id().as_ref())); + let scalars_dtype = Some(ext.scalars_dtype().write_flatbuffer(fbb)); let metadata = ext.metadata().map(|m| fbb.create_vector(m.as_ref())); fb::Extension::create( fbb, &fb::ExtensionArgs { id, + scalars_dtype, metadata, nullable: (*n).into(), }, diff --git a/vortex-dtype/src/serde/proto.rs b/vortex-dtype/src/serde/proto.rs index 6ffa8ae89c..f062f800f5 100644 --- a/vortex-dtype/src/serde/proto.rs +++ b/vortex-dtype/src/serde/proto.rs @@ -48,6 +48,11 @@ impl TryFrom<&pb::DType> for DType { DtypeType::Extension(e) => Ok(Self::Extension( ExtDType::new( ExtID::from(e.id.as_str()), + Arc::new(DType::try_from(e.scalars_dtype + .as_ref() + .ok_or_else(|| vortex_err!(InvalidSerde: "scalars_dtype must be provided in DType proto message"))? + .as_ref(), + ).map_err(|e| vortex_err!("failed converting DType from proto message: {}", e))?), e.metadata.as_ref().map(|m| ExtMetadata::from(m.as_ref())), ), e.nullable.into(), @@ -83,11 +88,12 @@ impl From<&DType> for pb::DType { element_type: Some(Box::new(l.as_ref().into())), nullable: (*n).into(), })), - DType::Extension(e, n) => DtypeType::Extension(pb::Extension { + DType::Extension(e, n) => DtypeType::Extension(Box::new(pb::Extension { id: e.id().as_ref().into(), + scalars_dtype: Some(Box::new(e.scalars_dtype().into())), metadata: e.metadata().map(|m| m.as_ref().into()), nullable: (*n).into(), - }), + })), }), } } diff --git a/vortex-flatbuffers/flatbuffers/vortex-dtype/dtype.fbs b/vortex-flatbuffers/flatbuffers/vortex-dtype/dtype.fbs index 8a31de56e0..b8d618e8e1 100644 --- a/vortex-flatbuffers/flatbuffers/vortex-dtype/dtype.fbs +++ b/vortex-flatbuffers/flatbuffers/vortex-dtype/dtype.fbs @@ -52,6 +52,7 @@ table List { table Extension { id: string; + scalars_dtype: DType; metadata: [ubyte]; nullable: bool; } diff --git a/vortex-flatbuffers/src/generated/array.rs b/vortex-flatbuffers/src/generated/array.rs index 5eed6a7c8a..58cad0616d 100644 --- a/vortex-flatbuffers/src/generated/array.rs +++ b/vortex-flatbuffers/src/generated/array.rs @@ -3,8 +3,8 @@ // @generated -use crate::scalar::*; use crate::dtype::*; +use crate::scalar::*; use core::mem; use core::cmp::Ordering; diff --git a/vortex-flatbuffers/src/generated/dtype.rs b/vortex-flatbuffers/src/generated/dtype.rs index b86459ec7a..264e0c09b9 100644 --- a/vortex-flatbuffers/src/generated/dtype.rs +++ b/vortex-flatbuffers/src/generated/dtype.rs @@ -1128,8 +1128,9 @@ impl<'a> flatbuffers::Follow<'a> for Extension<'a> { impl<'a> Extension<'a> { pub const VT_ID: flatbuffers::VOffsetT = 4; - pub const VT_METADATA: flatbuffers::VOffsetT = 6; - pub const VT_NULLABLE: flatbuffers::VOffsetT = 8; + pub const VT_SCALARS_DTYPE: flatbuffers::VOffsetT = 6; + pub const VT_METADATA: flatbuffers::VOffsetT = 8; + pub const VT_NULLABLE: flatbuffers::VOffsetT = 10; #[inline] pub unsafe fn init_from_table(table: flatbuffers::Table<'a>) -> Self { @@ -1142,6 +1143,7 @@ impl<'a> Extension<'a> { ) -> flatbuffers::WIPOffset> { let mut builder = ExtensionBuilder::new(_fbb); if let Some(x) = args.metadata { builder.add_metadata(x); } + if let Some(x) = args.scalars_dtype { builder.add_scalars_dtype(x); } if let Some(x) = args.id { builder.add_id(x); } builder.add_nullable(args.nullable); builder.finish() @@ -1156,6 +1158,13 @@ impl<'a> Extension<'a> { unsafe { self._tab.get::>(Extension::VT_ID, None)} } #[inline] + pub fn scalars_dtype(&self) -> Option> { + // Safety: + // Created from valid Table for this object + // which contains a valid value in this slot + unsafe { self._tab.get::>(Extension::VT_SCALARS_DTYPE, None)} + } + #[inline] pub fn metadata(&self) -> Option> { // Safety: // Created from valid Table for this object @@ -1179,6 +1188,7 @@ impl flatbuffers::Verifiable for Extension<'_> { use self::flatbuffers::Verifiable; v.visit_table(pos)? .visit_field::>("id", Self::VT_ID, false)? + .visit_field::>("scalars_dtype", Self::VT_SCALARS_DTYPE, false)? .visit_field::>>("metadata", Self::VT_METADATA, false)? .visit_field::("nullable", Self::VT_NULLABLE, false)? .finish(); @@ -1187,6 +1197,7 @@ impl flatbuffers::Verifiable for Extension<'_> { } pub struct ExtensionArgs<'a> { pub id: Option>, + pub scalars_dtype: Option>>, pub metadata: Option>>, pub nullable: bool, } @@ -1195,6 +1206,7 @@ impl<'a> Default for ExtensionArgs<'a> { fn default() -> Self { ExtensionArgs { id: None, + scalars_dtype: None, metadata: None, nullable: false, } @@ -1211,6 +1223,10 @@ impl<'a: 'b, 'b, A: flatbuffers::Allocator + 'a> ExtensionBuilder<'a, 'b, A> { self.fbb_.push_slot_always::>(Extension::VT_ID, id); } #[inline] + pub fn add_scalars_dtype(&mut self, scalars_dtype: flatbuffers::WIPOffset>) { + self.fbb_.push_slot_always::>(Extension::VT_SCALARS_DTYPE, scalars_dtype); + } + #[inline] pub fn add_metadata(&mut self, metadata: flatbuffers::WIPOffset>) { self.fbb_.push_slot_always::>(Extension::VT_METADATA, metadata); } @@ -1237,6 +1253,7 @@ impl core::fmt::Debug for Extension<'_> { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { let mut ds = f.debug_struct("Extension"); ds.field("id", &self.id()); + ds.field("scalars_dtype", &self.scalars_dtype()); ds.field("metadata", &self.metadata()); ds.field("nullable", &self.nullable()); ds.finish() diff --git a/vortex-flatbuffers/src/generated/message.rs b/vortex-flatbuffers/src/generated/message.rs index 17c43298a4..e9a123c1b8 100644 --- a/vortex-flatbuffers/src/generated/message.rs +++ b/vortex-flatbuffers/src/generated/message.rs @@ -3,8 +3,8 @@ // @generated -use crate::scalar::*; use crate::dtype::*; +use crate::scalar::*; use crate::array::*; use core::mem; use core::cmp::Ordering; diff --git a/vortex-proto/Cargo.toml b/vortex-proto/Cargo.toml index dbfac0c9c1..fd7102cf7c 100644 --- a/vortex-proto/Cargo.toml +++ b/vortex-proto/Cargo.toml @@ -22,5 +22,5 @@ dtype = [] scalar = ["dtype"] expr = ["dtype", "scalar"] -[lints] -workspace = true +#[lints] +#workspace = true diff --git a/vortex-proto/proto/dtype.proto b/vortex-proto/proto/dtype.proto index d4fe588428..eaa71f14a8 100644 --- a/vortex-proto/proto/dtype.proto +++ b/vortex-proto/proto/dtype.proto @@ -54,8 +54,9 @@ message List { message Extension { string id = 1; - optional bytes metadata = 2; - bool nullable = 3; + DType scalars_dtype = 2; + optional bytes metadata = 3; + bool nullable = 4; } message DType { diff --git a/vortex-proto/src/generated/vortex.dtype.rs b/vortex-proto/src/generated/vortex.dtype.rs index 9a3a956007..cadc67536e 100644 --- a/vortex-proto/src/generated/vortex.dtype.rs +++ b/vortex-proto/src/generated/vortex.dtype.rs @@ -1,14 +1,11 @@ // This file is @generated by prost-build. -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct Null {} -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct Bool { #[prost(bool, tag = "1")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct Primitive { #[prost(enumeration = "PType", tag = "1")] @@ -16,7 +13,6 @@ pub struct Primitive { #[prost(bool, tag = "2")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct Decimal { #[prost(uint32, tag = "1")] @@ -26,19 +22,16 @@ pub struct Decimal { #[prost(bool, tag = "3")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct Utf8 { #[prost(bool, tag = "1")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, Copy, PartialEq, ::prost::Message)] pub struct Binary { #[prost(bool, tag = "1")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Struct { #[prost(string, repeated, tag = "1")] @@ -48,7 +41,6 @@ pub struct Struct { #[prost(bool, tag = "3")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct List { #[prost(message, optional, boxed, tag = "1")] @@ -56,17 +48,17 @@ pub struct List { #[prost(bool, tag = "2")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Extension { #[prost(string, tag = "1")] pub id: ::prost::alloc::string::String, - #[prost(bytes = "vec", optional, tag = "2")] + #[prost(message, optional, boxed, tag = "2")] + pub scalars_dtype: ::core::option::Option<::prost::alloc::boxed::Box>, + #[prost(bytes = "vec", optional, tag = "3")] pub metadata: ::core::option::Option<::prost::alloc::vec::Vec>, - #[prost(bool, tag = "3")] + #[prost(bool, tag = "4")] pub nullable: bool, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct DType { #[prost(oneof = "d_type::DtypeType", tags = "1, 2, 3, 4, 5, 6, 7, 8, 9")] @@ -74,7 +66,6 @@ pub struct DType { } /// Nested message and enum types in `DType`. pub mod d_type { - #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum DtypeType { #[prost(message, tag = "1")] @@ -94,10 +85,9 @@ pub mod d_type { #[prost(message, tag = "8")] List(::prost::alloc::boxed::Box), #[prost(message, tag = "9")] - Extension(super::Extension), + Extension(::prost::alloc::boxed::Box), } } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Field { #[prost(oneof = "field::FieldType", tags = "1, 2")] @@ -105,7 +95,6 @@ pub struct Field { } /// Nested message and enum types in `Field`. pub mod field { - #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum FieldType { #[prost(string, tag = "1")] @@ -114,7 +103,6 @@ pub mod field { Index(u64), } } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct FieldPath { #[prost(message, repeated, tag = "1")] @@ -142,17 +130,17 @@ impl PType { /// (if the ProtoBuf definition does not change) and safe for programmatic use. pub fn as_str_name(&self) -> &'static str { match self { - PType::U8 => "U8", - PType::U16 => "U16", - PType::U32 => "U32", - PType::U64 => "U64", - PType::I8 => "I8", - PType::I16 => "I16", - PType::I32 => "I32", - PType::I64 => "I64", - PType::F16 => "F16", - PType::F32 => "F32", - PType::F64 => "F64", + Self::U8 => "U8", + Self::U16 => "U16", + Self::U32 => "U32", + Self::U64 => "U64", + Self::I8 => "I8", + Self::I16 => "I16", + Self::I32 => "I32", + Self::I64 => "I64", + Self::F16 => "F16", + Self::F32 => "F32", + Self::F64 => "F64", } } /// Creates an enum from field names used in the ProtoBuf definition. diff --git a/vortex-proto/src/generated/vortex.scalar.rs b/vortex-proto/src/generated/vortex.scalar.rs index 1cae9384e1..f79441ff51 100644 --- a/vortex-proto/src/generated/vortex.scalar.rs +++ b/vortex-proto/src/generated/vortex.scalar.rs @@ -1,5 +1,4 @@ // This file is @generated by prost-build. -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Scalar { #[prost(message, optional, tag = "1")] @@ -7,7 +6,6 @@ pub struct Scalar { #[prost(message, optional, tag = "2")] pub value: ::core::option::Option, } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct ScalarValue { #[prost(oneof = "scalar_value::Kind", tags = "1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12")] @@ -15,7 +13,6 @@ pub struct ScalarValue { } /// Nested message and enum types in `ScalarValue`. pub mod scalar_value { - #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum Kind { #[prost(enumeration = "::prost_types::NullValue", tag = "1")] @@ -42,7 +39,6 @@ pub mod scalar_value { ListValue(super::ListValue), } } -#[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct ListValue { #[prost(message, repeated, tag = "1")] diff --git a/vortex-proto/src/lib.rs b/vortex-proto/src/lib.rs index 41a1a9d178..19860c612d 100644 --- a/vortex-proto/src/lib.rs +++ b/vortex-proto/src/lib.rs @@ -1,14 +1,19 @@ +#![allow(clippy::all)] + #[cfg(feature = "dtype")] +#[allow(clippy::all)] #[rustfmt::skip] #[path = "./generated/vortex.dtype.rs"] pub mod dtype; #[cfg(feature = "scalar")] +#[allow(clippy::all)] #[rustfmt::skip] #[path = "./generated/vortex.scalar.rs"] pub mod scalar; #[cfg(feature = "expr")] +#[allow(clippy::all)] #[rustfmt::skip] #[path = "./generated/vortex.expr.rs"] pub mod expr; diff --git a/vortex-scalar/src/display.rs b/vortex-scalar/src/display.rs index b9dced5ecf..31922a90f3 100644 --- a/vortex-scalar/src/display.rs +++ b/vortex-scalar/src/display.rs @@ -59,6 +59,7 @@ impl Display for Scalar { } } DType::List(..) => todo!(), + // Specialized handling for date/time/timestamp builtin extension types. DType::Extension(dtype, _) if is_temporal_ext_type(dtype.id()) => { let metadata = TemporalMetadata::try_from(dtype).map_err(|_| std::fmt::Error)?; match ExtScalar::try_from(self) @@ -79,7 +80,18 @@ impl Display for Scalar { _ => Err(std::fmt::Error), } } - DType::Extension(..) => todo!(), + // Generic handling of unknown extension types. + // TODO(aduffy): Allow extension authors plugin their own Scalar display. + DType::Extension(..) => { + let scalar_value = ExtScalar::try_from(self) + .map_err(|_| std::fmt::Error)? + .value(); + if scalar_value.is_null() { + write!(f, "null") + } else { + write!(f, "{}", scalar_value) + } + } } } } @@ -237,6 +249,7 @@ mod tests { DType::Extension( ExtDType::new( TIME_ID.clone(), + Arc::new(PType::I32.into()), Some(ExtMetadata::from(TemporalMetadata::Time(TimeUnit::S))), ), Nullable, @@ -263,6 +276,7 @@ mod tests { DType::Extension( ExtDType::new( DATE_ID.clone(), + Arc::new(PType::I32.into()), Some(ExtMetadata::from(TemporalMetadata::Date(TimeUnit::D))), ), Nullable, @@ -302,6 +316,7 @@ mod tests { DType::Extension( ExtDType::new( TIMESTAMP_ID.clone(), + Arc::new(PType::I32.into()), Some(ExtMetadata::from(TemporalMetadata::Timestamp( TimeUnit::S, None, @@ -332,6 +347,7 @@ mod tests { DType::Extension( ExtDType::new( TIMESTAMP_ID.clone(), + Arc::new(PType::I64.into()), Some(ExtMetadata::from(TemporalMetadata::Timestamp( TimeUnit::S, Some(String::from("Pacific/Guam")), diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 69e8049229..835374adbb 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -39,7 +39,6 @@ fn execute_generate_proto() -> anyhow::Result<()> { let proto_files = vec![ vortex_proto.join("proto").join("dtype.proto"), vortex_proto.join("proto").join("scalar.proto"), - vortex_proto.join("proto").join("expr.proto"), ]; for file in &proto_files {